Files
siftlode/backend/app/routes/feed.py
T
peter 6116ad4bf4 refactor(feed): address /code-review high findings on the E2 stack
Review round on the E2 stack (S1-S5). No correctness bugs; the surviving
low/medium findings addressed:

- altitude (#1): extract the shared `tags_by_category` helper used by BOTH
  filtered_query (SQL tag WHERE) and feed_summary (Python facet/count), removing
  the category-grouping duplication, and add bidirectional "keep in lockstep —
  equivalence-tested by test_feed_summary_matches_count_plus_facets" comments so
  the SQL<->Python tag rule can't silently drift.
- test-coverage (#2): the /feed/summary equivalence test seeded only system tags,
  leaving the `ChannelTag.user_id == user.id` visibility branch untested. Add a
  user-owned Tag + link + two combos; the suite is now 346 (was 344) and the new
  cases pass (feed_summary agrees with /feed/count + /facets on user-visible tags).
- dead-code (#3): annotate get_feed_count, get_facets, and the FE api.feedCount/
  facets as "SUPERSEDED by /feed/summary — kept only as the equivalence oracle;
  retire together" so the intentionally-retained surface reads as deliberate.

Not changed (confirmed no-op): the search channel-name resolve (#4) is
once-per-request on a 633-row table and a deliberate pre-resolve for PERF-12's
BitmapOr plan; the facet loop (#5) already computes passes() once per
(channel, category) with a distinct exclude each, so there is no recomputation.

Gate green: ruff + 346 pytest; frontend tsc/knip/prettier (FE change is a comment).
2026-08-18 16:39:04 +02:00

404 lines
16 KiB
Python

from datetime import date, datetime, timezone
from fastapi import APIRouter, Depends, HTTPException, Query
from pydantic import BaseModel
from sqlalchemy import and_, func, or_, select
from sqlalchemy.dialects.postgresql import insert as pg_insert
from sqlalchemy.orm import Session
from sqlalchemy.orm.exc import StaleDataError
from app import quota
from app.auth import current_user
from app.db import get_db
from app.feedquery import (
decode_cursor,
encode_cursor,
feed_summary,
filtered_query,
keyset_predicate,
order_by,
serialize,
sort_keys,
)
from app.models import (
ChannelTag,
Tag,
User,
Video,
VideoState,
)
from app.sync.videos import parse_iso8601_duration
from app.utils import iso
from app.youtube.client import YouTubeClient, YouTubeError
router = APIRouter(prefix="/api", tags=["feed"])
# "saved" used to be a status; it's now membership in the built-in Watch later playlist.
VALID_STATES = {"new", "watched", "hidden"}
# Resume-position thresholds (mirror the client): positions below this are "didn't
# really start" and within this of the end are "basically finished" — neither is worth
# storing, so we clear the position instead (the video shows no progress bar).
# NB: the Plex player intentionally uses a LARGER finish margin (30s) because it marks
# long-form items watched, not just clears a resume bar — see routes/plex/playback.py.
PROGRESS_MIN_SECONDS = 5
FINISH_MARGIN_SECONDS = 10
# Shared query parameters for /feed and /feed/count.
def _feed_params(
tags: list[int] = Query(default=[]),
tag_mode: str = "or",
channel_ids: list[str] = Query(default=[]),
q: str | None = None,
min_duration: int | None = None,
max_duration: int | None = None,
max_age_days: int | None = None,
published_after: date | None = None,
published_before: date | None = None,
show_normal: bool = True,
include_shorts: bool = False,
include_live: bool = False,
show: str = "unwatched",
scope: str = "my",
library_source: str = "organic",
) -> dict:
return {
"tags": tags,
"tag_mode": tag_mode,
"channel_ids": channel_ids,
"q": q,
"min_duration": min_duration,
"max_duration": max_duration,
"max_age_days": max_age_days,
"published_after": published_after,
"published_before": published_before,
"show_normal": show_normal,
"include_shorts": include_shorts,
"include_live": include_live,
"show": show,
"scope": scope,
"library_source": library_source,
}
@router.get("/feed")
def get_feed(
params: dict = Depends(_feed_params),
sort: str = "newest",
seed: int = 0,
limit: int = Query(default=60, le=200),
cursor: str | None = None,
user: User = Depends(current_user),
db: Session = Depends(get_db),
) -> dict:
query, rank_expr = filtered_query(db, user, **params)
keys = sort_keys(sort, seed, rank_expr)
# Expose each key column on the row so we can build the next cursor from the last item.
query = query.add_columns(*[k["expr"].label(f"_k{i}") for i, k in enumerate(keys)])
query = query.order_by(*order_by(keys))
if cursor:
# The read-model raises a plain ValueError on a malformed cursor; translate it to the
# HTTP 400 here so feedquery stays web-framework-free.
try:
vals, cursor_id = decode_cursor(keys, cursor)
except ValueError:
# `from None`: the ValueError is an expected bad-input signal, not an error to chain.
raise HTTPException(status_code=400, detail="Invalid feed cursor") from None
for k, v in zip(keys, vals):
k["value"] = v
query = query.where(keyset_predicate(keys, cursor_id))
rows = db.execute(query.limit(limit + 1)).all()
has_more = len(rows) > limit
page = rows[:limit]
return {
"items": [serialize(r) for r in page],
"next_cursor": encode_cursor(keys, page[-1]) if (has_more and page) else None,
"has_more": has_more,
"limit": limit,
}
@router.get("/feed/count")
def get_feed_count(
params: dict = Depends(_feed_params),
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}
@router.get("/facets")
def get_facets(
params: dict = Depends(_feed_params),
user: User = Depends(current_user),
db: Session = Depends(get_db),
) -> dict:
"""Per-tag channel counts for the *current* filter context, so the sidebar can show
live counts and drop chips that no longer match anything. Each category is counted with
every other filter applied (scope, channel, date, content type, search, watch state,
and the other category's tags) but its own selections ignored — standard drill-down
faceting, so selecting one topic doesn't zero out the rest of the topics.
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).
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"):
# Disjunctive (OR) facets drop the category's own selections so its chips keep
# independent counts (you can OR more of them in). Conjunctive (AND, topics only)
# keeps them applied, so each remaining chip narrows to channels that ALSO have all
# already-selected topics — and tags that can't co-occur drop to zero (hidden).
conjunctive = category == "topic" and params.get("tag_mode") == "and"
base, _rank = filtered_query(
db,
user,
**{**params, "exclude_tag_category": None if conjunctive else category},
)
channels = base.with_only_columns(Video.channel_id).distinct().subquery()
rows = db.execute(
select(ChannelTag.tag_id, func.count(func.distinct(ChannelTag.channel_id)))
.select_from(channels)
.join(ChannelTag, ChannelTag.channel_id == channels.c.channel_id)
.join(Tag, and_(Tag.id == ChannelTag.tag_id, Tag.category == category))
.where(visible)
.group_by(ChannelTag.tag_id)
).all()
for tag_id, count in rows:
counts[tag_id] = count
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": <videos>, "facets": {<tag_id>: <channel_count>}} — 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
@router.post("/videos/{video_id}/state")
def set_video_state(
video_id: str,
body: VideoStateIn,
user: User = Depends(current_user),
db: Session = Depends(get_db),
) -> dict:
status = body.status
if status not in VALID_STATES:
raise HTTPException(status_code=400, detail=f"status must be one of {VALID_STATES}")
if db.get(Video, video_id) is None:
raise HTTPException(status_code=404, detail="Unknown video")
now = datetime.now(timezone.utc)
if status == "new":
# Un-marking: keep the row if it still holds a resume position (un-marking
# "watched" should restore the in-progress state, not wipe where the user left
# off); otherwise drop it. The in-app player fires this alongside a progress
# checkpoint that may DELETE the same row, so tolerate it vanishing under us.
row = db.execute(
select(VideoState).where(
VideoState.user_id == user.id, VideoState.video_id == video_id
)
).scalar_one_or_none()
if row is not None:
if row.position_seconds:
row.status = "new"
row.watched_at = None
else:
db.delete(row)
try:
db.commit()
except StaleDataError:
db.rollback()
return {"video_id": video_id, "status": "new"}
# watched / hidden: atomic upsert. The player marks "watched" at end-of-playback at
# the same instant a progress checkpoint may DELETE the in-progress row; a plain
# SELECT-then-UPDATE then raises StaleDataError ("0 rows matched") on that race. An
# INSERT … ON CONFLICT DO UPDATE keyed on uq_user_video is immune to it.
set_: dict = {"status": status, "updated_at": now}
if status == "watched":
set_["watched_at"] = now # hidden keeps any existing watched_at
stmt = (
pg_insert(VideoState)
.values(
user_id=user.id,
video_id=video_id,
status=status,
watched_at=now if status == "watched" else None,
)
.on_conflict_do_update(constraint="uq_user_video", set_=set_)
)
db.execute(stmt)
db.commit()
return {"video_id": video_id, "status": status}
@router.delete("/videos/{video_id}/state")
def clear_video_state(
video_id: str,
user: User = Depends(current_user),
db: Session = Depends(get_db),
) -> dict:
"""Reset a video to pristine for this user — drop the whole VideoState row (status,
watch position and watched_at), as if it had never been opened. Saved/playlist membership
lives elsewhere and is untouched."""
row = db.execute(
select(VideoState).where(
VideoState.user_id == user.id, VideoState.video_id == video_id
)
).scalar_one_or_none()
if row is not None:
db.delete(row)
try:
db.commit()
except StaleDataError:
db.rollback()
return {"video_id": video_id, "status": "new", "position_seconds": 0}
class VideoProgressIn(BaseModel):
# Accept a float and truncate (below) — the player may post a raw currentTime (a float), which
# the old `int(payload.get(...))` silently truncated. A plain `int` field would 422 that on the
# 10s progress hot path, losing the resume position; `float` keeps the old tolerance.
position_seconds: float = 0
duration_seconds: float = 0
@router.post("/videos/{video_id}/progress")
def set_video_progress(
video_id: str,
body: VideoProgressIn,
user: User = Depends(current_user),
db: Session = Depends(get_db),
) -> dict:
"""Checkpoint the in-app player's resume position (called periodically while playing).
Stores a per-user position on the video_states row without touching watch status, so a
partially-watched video can render a progress bar and match the "in progress" filter.
Trivially-early and near-finished positions clear the position rather than store it."""
position = int(body.position_seconds)
duration = int(body.duration_seconds)
if position < 0:
position = 0
if db.get(Video, video_id) is None:
raise HTTPException(status_code=404, detail="Unknown video")
# Decide whether this position is worth keeping (else clear it).
near_end = duration > 0 and position > duration - FINISH_MARGIN_SECONDS
keep = position >= PROGRESS_MIN_SECONDS and not near_end
row = db.execute(
select(VideoState).where(
VideoState.user_id == user.id, VideoState.video_id == video_id
)
).scalar_one_or_none()
if not keep:
# Nothing meaningful to store: drop a status-less row entirely, otherwise just
# zero the position (a saved/hidden video keeps its status).
if row is not None:
if row.status == "new":
db.delete(row)
else:
row.position_seconds = 0
row.progress_updated_at = datetime.now(timezone.utc)
# A concurrent state change (e.g. the end-of-playback "watched" mark) may
# have removed/updated this row already — tolerate the lost race.
try:
db.commit()
except StaleDataError:
db.rollback()
return {"video_id": video_id, "position_seconds": 0}
if row is None:
row = VideoState(user_id=user.id, video_id=video_id)
db.add(row)
row.position_seconds = position
row.progress_updated_at = datetime.now(timezone.utc)
db.commit()
return {"video_id": video_id, "position_seconds": position}
@router.get("/videos/{video_id}")
def get_video_detail(
video_id: str,
user: User = Depends(current_user),
db: Session = Depends(get_db),
) -> dict:
"""On-demand detail (description, like count) — kept out of the feed list payload
so the feed stays lean; fetched lazily, e.g. for the title hover popover.
Videos we already store are served from the DB for free. A video that isn't in
our DB (e.g. a YouTube link inside another video's description that the in-app
player navigated to) is resolved via the YouTube API (videos.list, 1 unit,
attributed to the requesting user)."""
v = db.get(Video, video_id)
if v is not None:
return {
"id": v.id,
"description": v.description,
"like_count": v.like_count,
"in_db": True,
"channel_id": v.channel_id,
"channel_title": v.channel.title if v.channel else None,
"published_at": iso(v.published_at),
"view_count": v.view_count,
"duration_seconds": v.duration_seconds,
}
# The DB path above is free; the off-catalog YouTube lookup below spends a quota unit, so the
# demo account can't reach it — it just sees "unknown" for a video we don't already store.
if user.is_demo:
raise HTTPException(status_code=404, detail="Unknown video")
try:
with quota.attribute(user.id, quota.QuotaAction.VIDEOS_LOOKUP), YouTubeClient(db, user) as yt:
items = yt.get_videos([video_id])
except YouTubeError as exc:
raise HTTPException(status_code=422, detail=f"YouTube lookup failed: {exc}")
if not items:
raise HTTPException(status_code=404, detail="Unknown video")
snippet = items[0].get("snippet", {})
stats = items[0].get("statistics", {})
likes = stats.get("likeCount")
views = stats.get("viewCount")
return {
"id": video_id,
"description": snippet.get("description"),
"like_count": int(likes) if likes is not None else None,
"in_db": False,
"channel_id": snippet.get("channelId"),
"channel_title": snippet.get("channelTitle"),
"published_at": snippet.get("publishedAt"),
"view_count": int(views) if views is not None else None,
"duration_seconds": parse_iso8601_duration(
items[0].get("contentDetails", {}).get("duration")
),
}