fix(e8): deep-review hardening + cut 0.73.0
Fixes from several multi-agent deep-review passes over the E8 feed-filtering & channel-manager epic (S1-S4): - feed_years buckets by UTC year to match the UTC-pinned year-filter bounds - tag suggestions: examples ∩ subscribed (drop tagged-then-unsubscribed channels from the profile denominator); exclude hidden channels as suggestion targets - channel manager: restore ghost chips + surface an error toast on a failed tag accept - year filter: debounce the search term so the per-year facet doesn't refetch per keystroke - channel tags: aria-label the "+N" overflow pills - extract useTagSuggestions hook; add backend + hook tests (profile boundaries, cap, language exclusion, unsubscribed/hidden handling, UTC bucketing, bulk-accept) Bump VERSION to 0.73.0 + release notes.
This commit is contained in:
@@ -450,7 +450,11 @@ def feed_years(db: Session, user: User, **params) -> list[dict]:
|
||||
**{**params, "published_after": None, "published_before": None, "max_age_days": None},
|
||||
)
|
||||
pub = base.with_only_columns(Video.published_at.label("pub")).subquery()
|
||||
year = func.extract("year", pub.c.pub)
|
||||
# Bucket by UTC year to stay in lockstep with the year filter's boundaries (published_after/before
|
||||
# are pinned to UTC in filtered_query). published_at is timestamptz, so a bare extract('year', …)
|
||||
# would use the DB session timezone and, on a non-UTC session, bucket a video near a year boundary
|
||||
# into a different year than the filter includes — the count and the drill-down feed would disagree.
|
||||
year = func.extract("year", func.timezone("UTC", pub.c.pub))
|
||||
rows = db.execute(
|
||||
select(year.label("y"), func.count()).group_by(year).order_by(year.desc())
|
||||
).all()
|
||||
|
||||
@@ -51,17 +51,25 @@ def compute_tag_suggestions(db: Session, user: User) -> dict[str, list[dict]]:
|
||||
capped at ``MAX_PER_CHANNEL``. Channels with no suggestion are omitted (empty dict when the
|
||||
user has no tag with a usable profile). Deterministic for a fixed DB state.
|
||||
"""
|
||||
# Candidate universe: the channels this user manages. A user can only tag a subscribed channel
|
||||
# (attach_tag requires a subscription), so example channels are a subset of this too — which
|
||||
# lets us bound the auto-tag scan to just these channels.
|
||||
subscribed: set[str] = set(
|
||||
db.scalars(select(Subscription.channel_id).where(Subscription.user_id == user.id)).all()
|
||||
)
|
||||
# Candidate universe: the channels this user is subscribed to. `subscribed` (incl. hidden) is the
|
||||
# SIGNAL set — a hidden channel is still a valid example / topic-feature source, since hiding only
|
||||
# affects the feed, not the tagging pattern. `hidden` is excluded as a suggestion TARGET below (the
|
||||
# Channel Manager shows ghost chips only on non-hidden rows, so suggesting for hidden channels would
|
||||
# inflate the header count past what's shown and let bulk-accept tag channels the user can't see).
|
||||
subs = db.execute(
|
||||
select(Subscription.channel_id, Subscription.hidden).where(Subscription.user_id == user.id)
|
||||
).all()
|
||||
subscribed: set[str] = {cid for cid, _ in subs}
|
||||
hidden: set[str] = {cid for cid, h in subs if h}
|
||||
if not subscribed:
|
||||
return {}
|
||||
|
||||
# The user's manual tag links: tag_id -> example channels, and each channel's user-tag set (so
|
||||
# a channel with ANY user tag is excluded from "untagged").
|
||||
# a channel with ANY user tag is excluded from "untagged"). Intersect with `subscribed`: unsubscribe
|
||||
# deletes the Subscription but NOT the source='user' ChannelTag, so a tagged-then-unsubscribed
|
||||
# channel would otherwise stay an "example" — inflating a profile's CHAR_SUPPORT denominator while
|
||||
# contributing zero features (features are scanned over `subscribed` only), dropping characteristic
|
||||
# topics. Keeping examples ⊆ subscribed makes the support fraction honest.
|
||||
examples: dict[int, set[str]] = {}
|
||||
user_tagged: set[str] = set()
|
||||
for tag_id, cid in db.execute(
|
||||
@@ -69,6 +77,8 @@ def compute_tag_suggestions(db: Session, user: User) -> dict[str, list[dict]]:
|
||||
ChannelTag.user_id == user.id, ChannelTag.source == "user"
|
||||
)
|
||||
).all():
|
||||
if cid not in subscribed:
|
||||
continue
|
||||
examples.setdefault(tag_id, set()).add(cid)
|
||||
user_tagged.add(cid)
|
||||
|
||||
@@ -104,7 +114,7 @@ def compute_tag_suggestions(db: Session, user: User) -> dict[str, list[dict]]:
|
||||
|
||||
suggestions: dict[str, list[dict]] = {}
|
||||
for cid in subscribed:
|
||||
if cid in user_tagged: # already has a user tag → not "untagged"
|
||||
if cid in hidden or cid in user_tagged: # hidden → no chips shown; tagged → not "untagged"
|
||||
continue
|
||||
cfeat = features.get(cid)
|
||||
if not cfeat:
|
||||
|
||||
@@ -253,6 +253,63 @@ def test_feed_years_counts_ignore_the_date_narrowing_but_honour_other_filters(
|
||||
]
|
||||
|
||||
|
||||
def test_feed_years_buckets_by_utc_year_under_a_non_utc_session(db, make_user):
|
||||
"""feed_years must bucket by UTC year to stay in lockstep with the year filter's UTC-pinned
|
||||
published_after/before bounds. published_at is timestamptz; a bare extract('year', …) would use the
|
||||
DB session timezone, so a video just before midnight UTC on New Year's Eve would bucket into the
|
||||
wrong (local) year on a non-UTC session and disagree with the feed the year chip drills into. Called
|
||||
directly on this session (the HTTP client uses a separate connection SET TIME ZONE wouldn't reach)."""
|
||||
from sqlalchemy import text
|
||||
|
||||
from app.feedquery import feed_years
|
||||
|
||||
user = make_user(email="tz-years@example.com")
|
||||
db.add(Channel(id="chTZ", title="chTZ", subscriber_count=1))
|
||||
db.add(Subscription(user_id=user.id, channel_id="chTZ", priority=0, hidden=False))
|
||||
# 2023-12-31 23:30 UTC — a positive-offset session tz local-shifts this into 2024.
|
||||
db.add(
|
||||
Video(
|
||||
id="tzv0",
|
||||
channel_id="chTZ",
|
||||
published_at=datetime(2023, 12, 31, 23, 30, tzinfo=timezone.utc),
|
||||
title="NYE clip",
|
||||
is_short=False,
|
||||
live_status="none",
|
||||
via_search=False,
|
||||
via_explore=False,
|
||||
)
|
||||
)
|
||||
db.commit()
|
||||
|
||||
params: dict = {
|
||||
"tags": [],
|
||||
"tag_modes": {"language": "or", "topic": "or", "other": "or"},
|
||||
"untagged": False,
|
||||
"channel_ids": [],
|
||||
"q": None,
|
||||
"min_duration": None,
|
||||
"max_duration": None,
|
||||
"max_age_days": None,
|
||||
"published_after": None,
|
||||
"published_before": None,
|
||||
"show_normal": True,
|
||||
"include_shorts": False,
|
||||
"include_live": False,
|
||||
"show": "all",
|
||||
"scope": "my",
|
||||
"library_source": "organic",
|
||||
}
|
||||
db.execute(
|
||||
text("SET TIME ZONE 'Asia/Tokyo'")
|
||||
) # +9: pushes the video's local wall-clock into 2024
|
||||
try:
|
||||
years = feed_years(db, user, **params)
|
||||
finally:
|
||||
db.execute(text("SET TIME ZONE 'UTC'")) # SET (not SET LOCAL) survives rollback — reset it
|
||||
# UTC bucketing keeps the boundary video in 2023; a bare session-tz extract would report 2024.
|
||||
assert years == [{"year": 2023, "count": 1}]
|
||||
|
||||
|
||||
@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,
|
||||
|
||||
@@ -14,6 +14,8 @@ Uses the DB lane (`db` fixture) — skipped when no Postgres is reachable.
|
||||
from app.models import Channel, ChannelTag, Subscription, Tag, User
|
||||
from app.sync.tagsuggest import (
|
||||
AUTOAPPLY_THRESHOLD,
|
||||
CHAR_SUPPORT,
|
||||
MAX_PER_CHANNEL,
|
||||
SUGGEST_THRESHOLD,
|
||||
compute_tag_suggestions,
|
||||
)
|
||||
@@ -26,10 +28,10 @@ def _auto(db, name, category="topic"):
|
||||
return tag
|
||||
|
||||
|
||||
def _subscribe(db, user, ch_id, *, auto_tag_ids=(), title=None):
|
||||
def _subscribe(db, user, ch_id, *, auto_tag_ids=(), title=None, hidden=False):
|
||||
"""A subscribed channel carrying the given auto-tags (source='auto', user_id NULL)."""
|
||||
db.add(Channel(id=ch_id, title=title or ch_id))
|
||||
db.add(Subscription(user_id=user.id, channel_id=ch_id))
|
||||
db.add(Subscription(user_id=user.id, channel_id=ch_id, hidden=hidden))
|
||||
for tid in auto_tag_ids:
|
||||
db.add(ChannelTag(channel_id=ch_id, tag_id=tid, user_id=None, source="auto"))
|
||||
|
||||
@@ -116,3 +118,169 @@ def test_partial_topic_overlap_is_a_fraction_of_the_profile(db):
|
||||
assert "UC_two_of_three" in out
|
||||
assert out["UC_two_of_three"][0]["tag_id"] == tag.id
|
||||
assert out["UC_two_of_three"][0]["confidence"] == round(2 / 3, 3)
|
||||
|
||||
|
||||
def test_language_alone_is_never_a_profile_even_when_it_is_the_only_shared_tag(db):
|
||||
# The anti-flood invariant WITH TEETH. Every example channel shares only a language; each carries a
|
||||
# DIFFERENT topic, so no topic reaches CHAR_SUPPORT → the topic-only profile is empty → no
|
||||
# suggestion. If language were ever admitted to the signal the profile would collapse to {language}
|
||||
# and the language-only channel would score 1.0 and be suggested — the exact flood the exclusion
|
||||
# prevents. (test_suggests_… can't catch a regression here: there the language merely enlarges an
|
||||
# already topic-driven profile, so removing the exclusion flips no assertion.)
|
||||
me = User(email="lang@example.com")
|
||||
db.add(me)
|
||||
db.flush()
|
||||
eng = _auto(db, "English", category="language")
|
||||
topics = [_auto(db, f"Topic{i}") for i in range(3)]
|
||||
lang = Tag(user_id=me.id, name="english-stuff", category="other")
|
||||
db.add(lang)
|
||||
db.flush()
|
||||
for i in range(3):
|
||||
cid = f"UC_lang_ex{i}"
|
||||
_subscribe(db, me, cid, auto_tag_ids=[eng.id, topics[i].id])
|
||||
db.add(ChannelTag(channel_id=cid, tag_id=lang.id, user_id=me.id, source="user"))
|
||||
_subscribe(db, me, "UC_lang_only", auto_tag_ids=[eng.id])
|
||||
db.commit()
|
||||
|
||||
# No characteristic topic → no profile → nothing suggested for anyone (language never carries it).
|
||||
assert compute_tag_suggestions(db, me) == {}
|
||||
|
||||
|
||||
def test_char_support_admits_a_two_thirds_topic_and_excludes_a_one_third_topic(db):
|
||||
# Pins the characteristic-profile threshold (CHAR_SUPPORT) at its PARTIAL-support boundary: across 3
|
||||
# example channels a topic in 2/3 (0.67 ≥ 0.6) enters the profile, one in 1/3 (0.33) does not. The
|
||||
# other fixtures give every topic full 3/3 support, so a regression demanding full support (or
|
||||
# flipping ≥ to >) would ship green — this is the only case that exercises fractional support.
|
||||
assert CHAR_SUPPORT == 0.6
|
||||
me = User(email="chr@example.com")
|
||||
db.add(me)
|
||||
db.flush()
|
||||
p, q, r = _auto(db, "P"), _auto(db, "Q"), _auto(db, "R")
|
||||
mix = Tag(user_id=me.id, name="mix", category="other")
|
||||
db.add(mix)
|
||||
db.flush()
|
||||
# p in 3/3 (kept), q in 2/3 (kept, 0.67), r in 1/3 (excluded, 0.33) → profile = {p, q}.
|
||||
example_topics = [[p.id, q.id, r.id], [p.id, q.id], [p.id]]
|
||||
for i, topics in enumerate(example_topics):
|
||||
cid = f"UC_mix_ex{i}"
|
||||
_subscribe(db, me, cid, auto_tag_ids=topics)
|
||||
db.add(ChannelTag(channel_id=cid, tag_id=mix.id, user_id=me.id, source="user"))
|
||||
_subscribe(db, me, "UC_pq", auto_tag_ids=[p.id, q.id]) # {p,q}/{p,q} = 1.0 → suggested
|
||||
_subscribe(db, me, "UC_only_p", auto_tag_ids=[p.id]) # 1/2 = 0.5 < 0.6 → dropped
|
||||
_subscribe(db, me, "UC_p_and_r", auto_tag_ids=[p.id, r.id]) # {p}/{p,q} = 0.5 → dropped
|
||||
db.commit()
|
||||
|
||||
out = compute_tag_suggestions(db, me)
|
||||
assert "UC_pq" in out and out["UC_pq"][0]["confidence"] == 1.0
|
||||
# q (2/3) IS in the profile: p alone covers only half of it. Would be a full 1.0 match if the
|
||||
# builder wrongly required 3/3 support (profile would collapse to {p}).
|
||||
assert "UC_only_p" not in out
|
||||
# r (1/3) is NOT in the profile: {p,r} covers only p. Would score 0.67 (suggested) if r wrongly
|
||||
# entered the profile (a downward drift of CHAR_SUPPORT).
|
||||
assert "UC_p_and_r" not in out
|
||||
|
||||
|
||||
def test_suggestions_are_capped_and_ordered_best_first(db):
|
||||
# A channel matching many user-tag profiles gets at most MAX_PER_CHANNEL suggestions, ordered best
|
||||
# score first, with the weakest dropped by the cap. No other fixture matches >1 profile, so the sort
|
||||
# and the cap are both no-ops there — dropping either ships green. Four DISTINCT confidences keep the
|
||||
# order unambiguous. (The exact-tie tag_id tie-break in the sort key is a determinism detail, not
|
||||
# asserted here: on a stable sort it isn't catchable without relying on unspecified DB row order.)
|
||||
me = User(email="cap@example.com")
|
||||
db.add(me)
|
||||
db.flush()
|
||||
|
||||
def profile_tag(name, n_topics):
|
||||
"""A user tag whose characteristic profile is exactly n_topics (3 identical example channels)."""
|
||||
topics = [_auto(db, f"{name}_t{i}") for i in range(n_topics)]
|
||||
tag = Tag(user_id=me.id, name=name, category="other")
|
||||
db.add(tag)
|
||||
db.flush()
|
||||
for i in range(3):
|
||||
cid = f"UC_{name}_ex{i}"
|
||||
_subscribe(db, me, cid, auto_tag_ids=[t.id for t in topics])
|
||||
db.add(ChannelTag(channel_id=cid, tag_id=tag.id, user_id=me.id, source="user"))
|
||||
return tag, topics
|
||||
|
||||
t0, top0 = profile_tag("p0", 1) # profile size 1
|
||||
t1, top1 = profile_tag("p1", 5) # profile size 5
|
||||
t2, top2 = profile_tag("p2", 10) # profile size 10
|
||||
t3, top3 = profile_tag("p3", 10) # profile size 10
|
||||
|
||||
# One untagged channel matching all four at distinct confidences:
|
||||
# t0: 1/1 = 1.00
|
||||
# t1: 4/5 = 0.80
|
||||
# t2: 7/10 = 0.70
|
||||
# t3: 6/10 = 0.60 → a valid hit (≥ SUGGEST_THRESHOLD) but the weakest, dropped by the cap of 3
|
||||
matched = (
|
||||
[top0[0].id]
|
||||
+ [t.id for t in top1[:4]]
|
||||
+ [t.id for t in top2[:7]]
|
||||
+ [t.id for t in top3[:6]]
|
||||
)
|
||||
_subscribe(db, me, "UC_many", auto_tag_ids=matched)
|
||||
db.commit()
|
||||
|
||||
hits = compute_tag_suggestions(db, me)["UC_many"]
|
||||
assert len(hits) == MAX_PER_CHANNEL # 4 matches, capped to 3
|
||||
assert [h["tag_id"] for h in hits] == [t0.id, t1.id, t2.id] # best score first
|
||||
assert [h["confidence"] for h in hits] == [1.0, 0.8, 0.7]
|
||||
assert t3.id not in [
|
||||
h["tag_id"] for h in hits
|
||||
] # the weakest match (0.60) is the one the cap drops
|
||||
|
||||
|
||||
def test_unsubscribed_example_channel_does_not_inflate_the_profile_denominator(db):
|
||||
# Unsubscribe deletes the Subscription but NOT the source='user' ChannelTag, so a tagged-then-
|
||||
# unsubscribed channel must NOT count as a profile example: it contributes no topic features (those
|
||||
# are scanned over subscribed channels only) yet would inflate CHAR_SUPPORT's denominator and drop a
|
||||
# genuinely characteristic topic. Here 'X' is characteristic at 2/3 of the SUBSCRIBED examples
|
||||
# (threshold 1.8, count 2 → kept), but if the unsubscribed 4th example counted, threshold 2.4 would
|
||||
# drop it and suppress the suggestion.
|
||||
me = User(email="unsub@example.com")
|
||||
db.add(me)
|
||||
db.flush()
|
||||
x, y = _auto(db, "X"), _auto(db, "Y")
|
||||
wood = Tag(user_id=me.id, name="wood", category="other")
|
||||
db.add(wood)
|
||||
db.flush()
|
||||
# 3 subscribed examples: 2 carry topic X, 1 carries only Y → X support 2/3 among subscribed.
|
||||
for cid, topic in [("UC_w0", x), ("UC_w1", x), ("UC_w2", y)]:
|
||||
_subscribe(db, me, cid, auto_tag_ids=[topic.id])
|
||||
db.add(ChannelTag(channel_id=cid, tag_id=wood.id, user_id=me.id, source="user"))
|
||||
# A 4th channel tagged "wood" but NO LONGER subscribed (channel + user tag, no Subscription row).
|
||||
db.add(Channel(id="UC_gone", title="UC_gone"))
|
||||
db.add(ChannelTag(channel_id="UC_gone", tag_id=wood.id, user_id=me.id, source="user"))
|
||||
_subscribe(
|
||||
db, me, "UC_cand", auto_tag_ids=[x.id]
|
||||
) # untagged X match → should be suggested "wood"
|
||||
db.commit()
|
||||
|
||||
out = compute_tag_suggestions(db, me)
|
||||
assert "UC_cand" in out
|
||||
assert out["UC_cand"][0]["tag_id"] == wood.id
|
||||
|
||||
|
||||
def test_hidden_subscriptions_are_not_suggestion_targets(db):
|
||||
# The Channel Manager renders suggestion ghost chips only on non-hidden rows, so a hidden channel
|
||||
# must not be a suggestion target — otherwise the header count / bulk-accept would span channels with
|
||||
# no visible chip. (Hidden channels still count as example/feature signal; only targeting is excluded.)
|
||||
me = User(email="hid@example.com")
|
||||
db.add(me)
|
||||
db.flush()
|
||||
z = _auto(db, "Z")
|
||||
ztag = Tag(user_id=me.id, name="ztag", category="other")
|
||||
db.add(ztag)
|
||||
db.flush()
|
||||
for i in range(3):
|
||||
cid = f"UC_z{i}"
|
||||
_subscribe(db, me, cid, auto_tag_ids=[z.id])
|
||||
db.add(ChannelTag(channel_id=cid, tag_id=ztag.id, user_id=me.id, source="user"))
|
||||
# A hidden, untagged, perfectly-matching subscription → NOT a target; a visible one → suggested.
|
||||
_subscribe(db, me, "UC_hidden", auto_tag_ids=[z.id], hidden=True)
|
||||
_subscribe(db, me, "UC_visible", auto_tag_ids=[z.id])
|
||||
db.commit()
|
||||
|
||||
out = compute_tag_suggestions(db, me)
|
||||
assert "UC_visible" in out
|
||||
assert "UC_hidden" not in out
|
||||
|
||||
Reference in New Issue
Block a user