From 995a493bd3d9012e7c066d39ce160498756999a8 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 6 Nov 2026 04:11:36 +0100 Subject: [PATCH 01/23] fix(playlists): the push side spends its own allowance, so a scripted loop cannot drain the shared YouTube budget push-plan reads the live list and push reads and writes it, both against the instance-wide budget, and neither had a limit. They now share one per-user allowance of 20 calls per 10 minutes, far above a person's plan-then-push, refused as playlist/push_cooldown with a sentence in both languages. --- backend/app/routes/playlists.py | 20 ++++++- backend/tests/test_abuse_limits.py | 64 +++++++++++++++++++++ frontend/src/i18n/locales/en/playlists.json | 1 + frontend/src/i18n/locales/hu/playlists.json | 1 + 4 files changed, 83 insertions(+), 3 deletions(-) diff --git a/backend/app/routes/playlists.py b/backend/app/routes/playlists.py index 5622dd2f..79ac345a 100644 --- a/backend/app/routes/playlists.py +++ b/backend/app/routes/playlists.py @@ -41,11 +41,16 @@ router = APIRouter(prefix="/api/playlists", tags=["playlists"]) # It guards EVERY route that spends under QuotaAction.PLAYLISTS_PULL, not just the bulk one: a # scripted loop over the single-playlist re-pull drains the same shared budget, and limiting one of # two routes with the same quota action is the asymmetric fix this codebase keeps re-learning. -# Deliberately NOT on the push side (`push-plan` / `push`): those run on every push attempt, -# including the no-op "already up to date" path, so a 3-per-10-minute allowance would refuse -# legitimate work. Their exposure is tracked as SEC-18 instead. _playlist_pull_limiter = RateLimiter(max_events=3, window_seconds=600) +# SEC-18, the push side: `push-plan` reads the live list (1 unit a page) and `push` reads and writes +# it, both against the same instance-wide budget. They get a budget of their own, counted in calls +# and far wider than the pull's: a press in the UI is one plan and, unless nothing changed, one push, +# so no person reaches it, while a scripted loop stops at it. A big push is not this limit's job — +# `push` already refuses one the day's remaining budget cannot cover. +PUSH_ALLOWANCE = 20 +_playlist_push_limiter = RateLimiter(max_events=PUSH_ALLOWANCE, window_seconds=600) + def _check_pull_cooldown(user: User) -> None: """Shared gate so the two PLAYLISTS_PULL routes cannot drift apart again.""" @@ -58,6 +63,7 @@ def _check_pull_cooldown(user: User) -> None: PlaylistRefusal = Literal[ "name_required", "not_on_youtube", + "push_cooldown", "quota_exceeded", "sync_all_busy", "sync_busy", @@ -244,6 +250,12 @@ def revert_youtube( raise youtube_refusal(exc, playlist=True) from exc +def _check_push_cooldown(user: User) -> None: + """Shared gate of the two push routes, so they spend one allowance and cannot drift apart.""" + if not _playlist_push_limiter.allow(str(user.id)): + raise _refused(429, "push_cooldown") + + def _pushable(db: Session, user: User, playlist_id: int) -> "Playlist": """A local playlist the user may push to YouTube (create or update). The built-in Watch later and read-only YouTube mirrors are never pushable.""" @@ -262,6 +274,7 @@ def push_plan( """Dry run: what a two-way 'Sync to YouTube' merge would do (what each side gains/loses) + the quota estimate, so the UI can show a confirm. For a linked playlist this reads the live YouTube state (cheap, 1 unit/page). Write scope is required — syncing is a YouTube write.""" + _check_push_cooldown(user) pl = _pushable(db, user, playlist_id) try: with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PUSH): @@ -282,6 +295,7 @@ def push( """Two-way sync a local playlist with YouTube (create it on first push, else 3-way merge so neither side loses an item). Refuses if the estimated quota exceeds what's left today, so a big sync can't strand half-applied. Write scope required (require_write_scope).""" + _check_push_cooldown(user) pl = _pushable(db, user, playlist_id) # One sync of a list at a time, across tabs, devices and the scheduler (`sync_claim`): the # client's own in-flight flag only covers the menus of one page. diff --git a/backend/tests/test_abuse_limits.py b/backend/tests/test_abuse_limits.py index 4333d269..bcdc499b 100644 --- a/backend/tests/test_abuse_limits.py +++ b/backend/tests/test_abuse_limits.py @@ -125,3 +125,67 @@ def test_the_single_playlist_repull_shares_the_pull_cooldown(db, as_user, make_u r = c.post("/api/playlists/sync-youtube") assert r.status_code == 429 assert r.json()["detail"] == {"code": "sync_cooldown"} + + +def _pushable_list(db, make_user, monkeypatch, plroutes): + """A local (never exported) list and a user with the write scope; the merge itself is stubbed — + the limiter is under test, not the YouTube round trip.""" + from app import auth + from app.reader.models import Playlist + + user = make_user() + pl = Playlist(user_id=user.id, name="Local", source="local") + db.add(pl) + db.commit() + monkeypatch.setattr(auth, "has_write_scope", lambda u: True) + monkeypatch.setattr(plroutes, "plan_merge", lambda d, u, p, live=None: {"units_estimate": 0}) + monkeypatch.setattr(plroutes, "merge_playlist", lambda d, u, p, live=None: {"added": 0}) + return user, pl + + +def test_the_push_side_goes_on_cooldown_after_its_allowance(db, as_user, make_user, monkeypatch): + # SEC-18: push-plan reads the live list and push writes it, both against the instance-wide + # YouTube budget. A press in the UI is one plan + one push, so the allowance is generous — but a + # scripted loop stops at it, with a reason the UI can say. + from app.routes import playlists as plroutes + + user, pl = _pushable_list(db, make_user, monkeypatch, plroutes) + c = as_user(user) + allowance = plroutes.PUSH_ALLOWANCE + codes = [c.get(f"/api/playlists/{pl.id}/push-plan").status_code for _ in range(allowance)] + assert codes == [200] * allowance + r = c.get(f"/api/playlists/{pl.id}/push-plan") + assert r.status_code == 429 + assert r.json()["detail"] == {"code": "playlist", "reason": "push_cooldown"} + + +def test_plan_and_push_draw_on_one_allowance(db, as_user, make_user, monkeypatch): + # The asymmetric fix again, avoided up front: a limit on one of the two routes leaves the same + # drain open next door. + from app.routes import playlists as plroutes + + user, pl = _pushable_list(db, make_user, monkeypatch, plroutes) + c = as_user(user) + for i in range(plroutes.PUSH_ALLOWANCE): + if i % 2: + r = c.get(f"/api/playlists/{pl.id}/push-plan") + else: + r = c.post(f"/api/playlists/{pl.id}/push") + assert r.status_code == 200, (i, r.json()) + r = c.post(f"/api/playlists/{pl.id}/push") + assert r.status_code == 429 + assert r.json()["detail"] == {"code": "playlist", "reason": "push_cooldown"} + + +def test_the_push_cooldown_is_per_user(db, as_user, make_user, monkeypatch): + from app.routes import playlists as plroutes + + a, pa = _pushable_list(db, make_user, monkeypatch, plroutes) + b, pb = _pushable_list(db, make_user, monkeypatch, plroutes) + ca = as_user(a) + codes = [ + ca.get(f"/api/playlists/{pa.id}/push-plan").status_code + for _ in range(plroutes.PUSH_ALLOWANCE + 1) + ] + assert codes == [200] * plroutes.PUSH_ALLOWANCE + [429] + assert as_user(b).get(f"/api/playlists/{pb.id}/push-plan").status_code == 200 diff --git a/frontend/src/i18n/locales/en/playlists.json b/frontend/src/i18n/locales/en/playlists.json index 619e565a..dd7574e4 100644 --- a/frontend/src/i18n/locales/en/playlists.json +++ b/frontend/src/i18n/locales/en/playlists.json @@ -89,6 +89,7 @@ "watch_later_not_pushable": "The Watch later list can't be sent to YouTube", "watch_later_undeletable": "The Watch later list can't be deleted", "youtube_editing_off": "Enable YouTube editing in Settings first", + "push_cooldown": "You've synced to YouTube many times just now — give it a few minutes", "quota_exceeded": "Not enough YouTube quota left today for this sync", "sync_busy": "This playlist is already syncing with YouTube — try again when it finishes", "sync_all_busy": "Your playlists are already syncing with YouTube — try again when it finishes" diff --git a/frontend/src/i18n/locales/hu/playlists.json b/frontend/src/i18n/locales/hu/playlists.json index 6c7c4aab..2b84fbbf 100644 --- a/frontend/src/i18n/locales/hu/playlists.json +++ b/frontend/src/i18n/locales/hu/playlists.json @@ -89,6 +89,7 @@ "watch_later_not_pushable": "A „Megnézem később” listát nem lehet a YouTube-ra küldeni", "watch_later_undeletable": "A „Megnézem később” lista nem törölhető", "youtube_editing_off": "Előbb engedélyezd a YouTube-szerkesztést a Beállításokban", + "push_cooldown": "Az imént sokszor szinkronizáltál a YouTube-ra — várj pár percet", "quota_exceeded": "Mára nem maradt elég YouTube-keret ehhez a szinkronhoz", "sync_busy": "Ez a lista épp szinkronizál a YouTube-bal — próbáld újra, ha végzett", "sync_all_busy": "A listáid épp szinkronizálnak a YouTube-bal — próbáld újra, ha végzett" From 4e4fff87d488a3f3237dfa93d38f13089eacc1da Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 6 Nov 2026 04:54:56 +0100 Subject: [PATCH 02/23] fix(downloads): a user's download is judged at every connection it makes, not only at the URL typed The URL guard judged the address the user typed; a public host's 3xx into our network, a page's formats pointing inside, and a name rebound between the check and the fetch all got past it, and the stored error told a live internal port from a dead one. Every non-YouTube download now runs through an egress gate on the worker's loopback: it resolves each host once, refuses any forbidden answer before dialling, and connects to the address it vetted. Measured on the worker image: yt-dlp's proxy reaches the Requests and CurlCFFI handlers, native HLS and ffmpeg alike. Formats fetched over anything but http(s), which ffmpeg would dial directly, are dropped, and one factory opens every yt-dlp so none skips it. --- backend/.basedpyright/baseline.json | 26 -- backend/app/downloads/adapters.py | 10 +- backend/app/downloads/backfill.py | 11 +- backend/app/downloads/egress.py | 264 +++++++++++++++ backend/app/downloads/formats.py | 16 +- backend/app/downloads/pipeline.py | 6 +- backend/app/downloads/urlguard.py | 26 +- backend/tests/test_clip.py | 3 + backend/tests/test_egress.py | 322 +++++++++++++++++++ backend/tests/test_facebook_impersonation.py | 3 + backend/tests/test_facebook_titles.py | 5 +- backend/tests/test_failure_lines.py | 14 +- backend/tests/test_formats.py | 23 +- backend/tests/test_ytdlp_logging.py | 4 +- 14 files changed, 668 insertions(+), 65 deletions(-) create mode 100644 backend/app/downloads/egress.py create mode 100644 backend/tests/test_egress.py diff --git a/backend/.basedpyright/baseline.json b/backend/.basedpyright/baseline.json index 8af825b4..af71727c 100644 --- a/backend/.basedpyright/baseline.json +++ b/backend/.basedpyright/baseline.json @@ -364,25 +364,7 @@ } } ], - "./app/downloads/adapters.py": [ - { - "code": "reportArgumentType", - "range": { - "startColumn": 30, - "endColumn": 38, - "lineCount": 1 - } - } - ], "./app/downloads/backfill.py": [ - { - "code": "reportArgumentType", - "range": { - "startColumn": 34, - "endColumn": 38, - "lineCount": 1 - } - }, { "code": "reportReturnType", "range": { @@ -413,14 +395,6 @@ } ], "./app/downloads/pipeline.py": [ - { - "code": "reportArgumentType", - "range": { - "startColumn": 34, - "endColumn": 38, - "lineCount": 1 - } - }, { "code": "reportAttributeAccessIssue", "range": { diff --git a/backend/app/downloads/adapters.py b/backend/app/downloads/adapters.py index ee693f6c..1a932fb3 100644 --- a/backend/app/downloads/adapters.py +++ b/backend/app/downloads/adapters.py @@ -14,10 +14,10 @@ Resolution works both before extraction (from source_kind / URL host) and after import re from urllib.parse import urlparse -import yt_dlp from yt_dlp.networking.impersonate import ImpersonateTarget from app.config import settings +from app.downloads import egress from app.downloads.stated import StatedFailure # Facebook's "Cannot parse data": the page yt-dlp got was a wall, not the content. Both the Chrome retry @@ -33,6 +33,9 @@ class SourceAdapter: """Generic source: yt-dlp handles everything and the ref is already a fetchable URL.""" key = "generic" + # SEC-17: every fetch of this source's download goes through the egress gate (`egress`). A + # source's own host list is no guarantee — its pages, redirects and formats can point anywhere. + egress_guarded = True # --- extraction (worker._extract_with_fallback / backfill) --------------------------------- def resolve_url(self, url: str, ydl_opts: dict) -> str: @@ -92,6 +95,9 @@ class SourceAdapter: class YouTubeAdapter(SourceAdapter): key = "youtube" + # Not through the gate: YouTube's fetches go to Google's fixed hosts, and the PO-token provider + # it needs is the sidecar on our internal network, which the gate exists to refuse. + egress_guarded = False # Errors worth retrying with a different player-client set (YouTube per-client flakiness). _RETRIABLE = ( "not a bot", @@ -179,7 +185,7 @@ class FacebookAdapter(SourceAdapter): post_id = facebook_group_post_id(url) if post_id is None or not ydl_opts.get("cookiefile"): return url - with yt_dlp.YoutubeDL(ydl_opts) as ydl: + with egress.open_ydl(ydl_opts) as ydl: raw = ydl.urlopen(url).read(self._POST_PAGE_MAX_BYTES) page = raw.decode("utf-8", "replace") # Measured: a refused or expired cookie gets the login page (~450 KB, a login diff --git a/backend/app/downloads/backfill.py b/backend/app/downloads/backfill.py index a79ed4e2..ca0cc804 100644 --- a/backend/app/downloads/backfill.py +++ b/backend/app/downloads/backfill.py @@ -16,7 +16,6 @@ Run once per environment: from urllib.parse import urlsplit -import yt_dlp from sqlalchemy import select, update from sqlalchemy.orm import Session @@ -25,7 +24,7 @@ from app.config import settings from app.db import SessionLocal from app.downloads import adapters from app.downloads import edit as editmod -from app.downloads import formats, service +from app.downloads import egress, formats, service from app.downloads.models import DownloadJob, MediaAsset from app.models import DISPLAY_TEXT_MAX from app.titles import normalize_title @@ -43,11 +42,15 @@ def extract_meta(url: str, adapter: adapters.SourceAdapter, asset_id: int) -> di def _extract_meta_scoped(url: str, adapter: adapters.SourceAdapter, asset_id: int) -> dict | None: for clients in adapter.client_sets(): opts = formats.build_ydl_opts( - {}, "%(id)s.%(ext)s", lambda d: None, extra=adapter.ydl_opts_extra(clients) + {}, + "%(id)s.%(ext)s", + lambda d: None, + extra=adapter.ydl_opts_extra(clients), + egress=adapter.egress_guarded, ) opts["skip_download"] = True try: - with yt_dlp.YoutubeDL(opts) as ydl: + with egress.open_ydl(opts) as ydl: info = ydl.extract_info(url, download=False) if info: return info diff --git a/backend/app/downloads/egress.py b/backend/app/downloads/egress.py new file mode 100644 index 00000000..991e8ce8 --- /dev/null +++ b/backend/app/downloads/egress.py @@ -0,0 +1,264 @@ +"""The egress gate: every fetch of a user-supplied download goes through here, and this is where the +address is judged — at the moment of the connection, not when the URL was typed (SEC-17). + +`urlguard` judges the URL the user typed, at enqueue and again in the worker. What that cannot see is +every request yt-dlp makes after it: a public host answering 3xx with an internal `Location`, a page +whose formats point inside, a name that resolves public for the check and private for the fetch (DNS +rebinding). So a guarded download runs with yt-dlp's `proxy` set to this gate, a small HTTP proxy on +the worker's loopback. For each request it resolves the host ONCE, refuses the connection if any +address the name has is one the server must not fetch from (`urlguard.is_forbidden`), and otherwise +connects to the very address it vetted — there is no second lookup for a rebinding answer to slip into. + +Measured (yt-dlp 2026.8.19, 2026-11-06) on the worker image, with a logging proxy: the `proxy` option +reaches every path a download takes — the Requests handler, the CurlCFFI one an impersonated request +uses, the native HLS downloader and an ffmpeg HLS download (segments included). ffmpeg itself also +speaks rtmp, rtsp, ftp, tcp and more, which no HTTP proxy covers, so a guarded download keeps only the +formats it fetches over http(s) (`HttpOnlyFormats`, added by `open_ydl`) and fetches HLS natively. + +A refused connection is answered `403` before anything is dialled, so a live internal port and a dead +one read the same; `errors.user_error_text` then stores the one redacted sentence for both. + +Not used for YouTube: those downloads talk to Google's fixed hosts and to the PO-token sidecar, which +lives on the internal network on purpose (`adapters.YouTubeAdapter.egress_guarded`). +""" + +import ipaddress +import select +import socket +import socketserver +import threading +from collections.abc import Callable +from typing import Any, cast +from urllib.parse import urlsplit + +import yt_dlp +from yt_dlp.postprocessor.common import PostProcessor + +from app import logs +from app.downloads import urlguard + +log = logs.get_logger(__name__) + +Resolver = Callable[..., list] +Forbidden = Callable[[ipaddress.IPv4Address | ipaddress.IPv6Address], bool] + +_BLOCKED = b"HTTP/1.1 403 Forbidden\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" +_BAD_GATEWAY = b"HTTP/1.1 502 Bad Gateway\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" +_BAD_REQUEST = b"HTTP/1.1 400 Bad Request\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" +_MAX_HEAD = 64 * 1024 +# A tunnel with no traffic either way for this long is closed. Above yt-dlp's own socket_timeout +# (30 s, `formats.build_ydl_opts`), so the gate never ends a transfer yt-dlp would still wait for. +_IDLE_S = 120 +# Hop-by-hop headers a proxy must not pass on; `Connection: close` replaces them, so one connection +# carries one request and a reused client connection can never be steered to a second host unvetted. +_HOP = (b"proxy-", b"connection:", b"keep-alive:") + + +def vetted_address( + host: str, port: int, *, resolve: Resolver = socket.getaddrinfo, forbidden: Forbidden +) -> tuple[str, int] | None: + """The address to connect to for `host:port`, or None when it must not be fetched: the name has + no answer, or ANY of its addresses is forbidden (a round-robin answer is not a coin flip). IPv4 + only, as every download is (`source_address` 0.0.0.0 in `formats.build_ydl_opts`).""" + try: + infos = resolve(host, port, socket.AF_INET, socket.SOCK_STREAM) + except (socket.gaierror, UnicodeError, OSError): + # silent-ok: "no answer" is a refusal, and the caller's refusal is the line that is logged. + return None + addrs = [info[4] for info in infos] + if not addrs: + return None + for addr in addrs: + try: + ip = ipaddress.ip_address(addr[0]) + except ValueError: + # silent-ok: an answer that is not an address is refused like a forbidden one, and the + # caller logs the refusal. + return None + if forbidden(ip): + return None + return addrs[0][0], addrs[0][1] + + +def _read_head(sock: socket.socket) -> bytes | None: + buf = b"" + while b"\r\n\r\n" not in buf: + if len(buf) > _MAX_HEAD: + return None + chunk = sock.recv(8192) + if not chunk: + return None + buf += chunk + return buf + + +def _pipe(a: socket.socket, b: socket.socket) -> None: + socks = [a, b] + while True: + ready, _, _ = select.select(socks, [], [], _IDLE_S) + if not ready: + return + for s in ready: + data = s.recv(65536) + if not data: + return + (b if s is a else a).sendall(data) + + +class _Handler(socketserver.BaseRequestHandler): + def handle(self) -> None: + client: socket.socket = self.request + head = _read_head(client) + if head is None: + return + head_part, _, body = head.partition(b"\r\n\r\n") + lines = head_part.split(b"\r\n") + try: + method, target, version = lines[0].decode("latin1").split(" ", 2) + except ValueError: + # silent-ok: the only client is yt-dlp on this loopback; a malformed request is answered + # 400 and reaches its own error, which the job records. + client.sendall(_BAD_REQUEST) + return + if method == "CONNECT": + host, _, port_s = target.rpartition(":") + host = host.strip("[]") + upstream = self._dial(host, port_s) + if upstream is None: + return + with upstream: + client.sendall(b"HTTP/1.1 200 Connection established\r\n\r\n") + if body: + upstream.sendall(body) + _pipe(client, upstream) + return + u = urlsplit(target) + if u.scheme != "http" or not u.hostname: + client.sendall(_BLOCKED) + return + try: + port = u.port or 80 + except ValueError: + # silent-ok: as for a malformed request line above — answered 400, recorded by the job. + client.sendall(_BAD_REQUEST) + return + upstream = self._dial(u.hostname, str(port)) + if upstream is None: + return + path = (u.path or "/") + (f"?{u.query}" if u.query else "") + headers = [h for h in lines[1:] if not h.lower().startswith(_HOP)] + request = b"\r\n".join( + [f"{method} {path} {version}".encode("latin1"), *headers, b"Connection: close"] + ) + with upstream: + upstream.sendall(request + b"\r\n\r\n" + body) + _pipe(client, upstream) + + def _dial(self, host: str, port_s: str) -> socket.socket | None: + """Connect to the address `host` was vetted at, or answer the client and return None.""" + client: socket.socket = self.request + try: + port = int(port_s) + except ValueError: + # silent-ok: as for a malformed request line above — answered 400, recorded by the job. + client.sendall(_BAD_REQUEST) + return None + gate = cast("_Gate", self.server) + addr = vetted_address(host, port, resolve=gate.resolve, forbidden=gate.forbidden) + if addr is None: + log.warning( + "egress refused a connection to %s:%s", + host, # log-pii-ok: the host a download tried to reach, never its path or query + port, + extra=logs.event("download.egress.refused", port=port), + ) + client.sendall(_BLOCKED) + return None + try: + return socket.create_connection(addr, timeout=_IDLE_S) + except OSError: + # silent-ok: a public host that does not answer; yt-dlp reports it as the job's own + # network failure, which the job records. + client.sendall(_BAD_GATEWAY) + return None + + +class _Gate(socketserver.ThreadingTCPServer): + daemon_threads = True + allow_reuse_address = True + + def __init__(self, resolve: Resolver, forbidden: Forbidden): + self.resolve = resolve + self.forbidden = forbidden + super().__init__(("127.0.0.1", 0), _Handler) + + @property + def url(self) -> str: + return f"http://127.0.0.1:{self.server_address[1]}" + + +def start_gate( + *, resolve: Resolver = socket.getaddrinfo, forbidden: Forbidden = urlguard.is_forbidden +) -> _Gate: + """A gate serving on its own thread. The worker uses the one `proxy_url` starts; a test starts + its own, with a resolver and a judgement it controls.""" + gate = _Gate(resolve, forbidden) + threading.Thread(target=gate.serve_forever, name="egress-gate", daemon=True).start() + return gate + + +# The options key `formats.build_ydl_opts` marks a guarded download with; `open_ydl` reads it. yt-dlp +# keeps unknown keys in its params and never acts on them. +GUARDED = "siftlode_egress_guarded" + + +def open_ydl(opts: dict) -> yt_dlp.YoutubeDL: + """The one way this package opens yt-dlp (a contract test holds it): a guarded download also + gets `HttpOnlyFormats`, which yt-dlp takes only as an instance, not through the options dict.""" + # yt-dlp's stub types its params as a TypedDict; these are the plain dict `formats.build_ydl_opts` + # assembles, the same every caller handed yt-dlp before. + ydl = yt_dlp.YoutubeDL(opts) # type: ignore[arg-type] + if opts.get(GUARDED): + ydl.add_post_processor(HttpOnlyFormats(ydl), when="pre_process") + return ydl + + +_gate: _Gate | None = None +_gate_lock = threading.Lock() + + +def proxy_url() -> str: + """This process's gate, started on first use (the worker's jobs share it).""" + global _gate + with _gate_lock: + if _gate is None: + _gate = start_gate() + return _gate.url + + +# The protocols yt-dlp fetches over http(s) — the only ones the gate sees. Everything else (rtmp, +# rtsp, mms, f4m's own fetcher, ws) would be dialled by ffmpeg or another tool directly. +_HTTP_PROTOCOLS = frozenset({"http", "https", "m3u8_native", "http_dash_segments"}) + + +class HttpOnlyFormats(PostProcessor): + """Drops every format a guarded download would fetch past the gate. Runs as `pre_process`, which + yt-dlp re-reads its formats after ("The pre-processors may have modified the formats"), so the + selector never sees them. With nothing left, yt-dlp reports no formats — a refusal, not a leak.""" + + def run(self, information): + # yt-dlp's stub types the info dict as a TypedDict with optional keys; it is a dict here. + info = cast("dict[str, Any]", information) + if info.get("formats"): + info["formats"] = [f for f in info["formats"] if _over_http(f)] + elif info.get("url") and not _over_http(info): + info["url"] = None + return [], information + + +def _over_http(fmt: dict) -> bool: + url = fmt.get("manifest_url") or fmt.get("url") or "" + if urlsplit(url).scheme not in ("http", "https"): + return False + protocol = fmt.get("protocol") or urlsplit(fmt.get("url") or "").scheme + return all(p in _HTTP_PROTOCOLS for p in protocol.split("+")) diff --git a/backend/app/downloads/formats.py b/backend/app/downloads/formats.py index 959753c9..95066371 100644 --- a/backend/app/downloads/formats.py +++ b/backend/app/downloads/formats.py @@ -28,6 +28,7 @@ import urllib.parse from collections.abc import Iterator from app import logs +from app.downloads import egress as egress_gate log = logs.get_logger(__name__) @@ -294,6 +295,8 @@ def build_ydl_opts( extra: dict | None = None, cookiefile: str | None = None, max_bytes: int | None = None, + *, + egress: bool, ) -> dict: """Assemble yt-dlp options for a single download to `outtmpl` (a full path template). @@ -309,7 +312,11 @@ def build_ydl_opts( a file announced larger than it rather than filling the disk and only then being caught by the quota accounting. Its ffmpeg downloader (a `download_ranges` fetch) does not read it, nor does a fragmented download (YouTube's formats: measured, 718 MB fetched with 1 MB left) — so the - pipeline also holds every produced file to what is left by its real size.""" + pipeline also holds every produced file to what is left by its real size. + + `egress` (SEC-17) routes every fetch through the worker's egress gate, which refuses a connection + into our own network, and keeps only the formats fetched over http(s). Required, with no default: + a new caller has to say which kind of download it builds (`SourceAdapter.egress_guarded`).""" s = normalize(spec) h = s["max_height"] opts: dict = { @@ -364,6 +371,13 @@ def build_ydl_opts( if cookiefile: opts["cookiefile"] = cookiefile + # SEC-17. AFTER the adapter merge, like the cookie: an adapter must not be able to point a + # guarded download past the gate. + if egress: + opts["proxy"] = egress_gate.proxy_url() + opts["hls_prefer_native"] = True + opts[egress_gate.GUARDED] = True + if s["mode"] == "a": opts["format"] = "bestaudio/best" opts["postprocessors"].append( diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index 236b162d..cecda260 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -17,7 +17,7 @@ from app.downloads import clip as clipmod from app.downloads import edit as editmod from app.downloads import errors as dlerrors from app.downloads import adapters, credentials, formats, mediakind, playable, probe, quota -from app.downloads import backfill +from app.downloads import backfill, egress from app.downloads import service, storage from app.downloads import urlguard from app.downloads.jobdb import ( @@ -394,6 +394,7 @@ def _extract_with_fallback( _make_progress_hook(job_id, asset_id), cookiefile=cookiefile, max_bytes=max_bytes, + egress=adapter.egress_guarded, ), ) last_exc: Exception | None = None @@ -405,6 +406,7 @@ def _extract_with_fallback( extra=adapter.ydl_opts_extra(clients), cookiefile=cookiefile, max_bytes=max_bytes, + egress=adapter.egress_guarded, ) opts["postprocessor_hooks"] = [_make_pp_hook(job_id)] if section is not None: @@ -415,7 +417,7 @@ def _extract_with_fallback( # (Per-ATTEMPT only; a single extract_info that itself runs past the stall timeout is still # killed — acceptable, as socket_timeout + extractor_retries bound one attempt well under it.) try: - with yt_dlp.YoutubeDL(opts) as ydl: + with egress.open_ydl(opts) as ydl: return ydl.extract_info(target, download=True) except yt_dlp.utils.DownloadError as exc: last_exc = exc diff --git a/backend/app/downloads/urlguard.py b/backend/app/downloads/urlguard.py index 854aa732..69e86e8a 100644 --- a/backend/app/downloads/urlguard.py +++ b/backend/app/downloads/urlguard.py @@ -10,22 +10,12 @@ What this closes: a URL whose host resolves to a loopback / private / link-local is refused, at enqueue AND again in the worker before extraction. Both, because DNS can answer differently between the two moments, and the worker is where the socket is actually opened. -What it does NOT close, stated plainly rather than implied, because both need the same fix — gating -the connection itself with a custom socket factory inside yt-dlp, a much larger change than this: - -* DNS rebinding: a host that passes the worker's check and then answers yt-dlp's own resolution - with an internal address. -* HTTP redirects: a public host that answers 3xx with an internal ``Location``. yt-dlp follows it, - and only the URL the user TYPED was ever judged here. An attacker-controlled host or any public - open redirect reaches it, so this is the cheaper of the two. It does NOT usually yield the - response body — a non-media body fails the mp4 remux and stores nothing — but it does hand back - the readout above: the redaction in ``errors.user_error_text`` fires only for the ``network`` - class, so a dead internal port (refused → ``network`` → redacted) is distinguishable from a live - one (an HTTP answer that is not media → ``unsupported_host``/``format_unavailable``, raw text - kept). Tracked as SEC-17. - -The two checks here stop the straightforward case (typing ``http://127.0.0.1:6379`` or an internal -hostname into the box), which is the reachable one. +What this alone cannot see is every request yt-dlp makes AFTER the typed URL: a public host +answering 3xx with an internal ``Location``, and DNS rebinding (a name that passes this check and +then resolves inside for yt-dlp's own lookup). Those are closed one layer down, by `egress`: a guarded +download's every connection goes through a gate that judges the address it actually dials, with +this module's `is_forbidden`. The checks here stay as the early, structured refusal — a user typing +``http://127.0.0.1:6379`` hears why at once instead of from a failed job. Not used for YouTube jobs: those resolve to a fixed public host from an 11-char id, so there is no user-controlled destination to guard. @@ -62,7 +52,7 @@ class BlockedUrl(Exception): self.reason: UrlBlockedReason = reason -def _is_forbidden(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool: +def is_forbidden(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool: """Every address family we must not open a connection to. Checked explicitly rather than leaning on ``is_private`` alone, whose coverage differs between IPv4 and IPv6.""" # ::ffff:127.0.0.1 is loopback wearing an IPv6 hat — unwrap before judging it. @@ -113,5 +103,5 @@ def check_url(raw: str) -> None: ip = ipaddress.ip_address(addr) except ValueError: # getaddrinfo returned something unparseable; treat as unusable raise BlockedUrl(REASON_PRIVATE) from None - if _is_forbidden(ip): + if is_forbidden(ip): raise BlockedUrl(REASON_PRIVATE) diff --git a/backend/tests/test_clip.py b/backend/tests/test_clip.py index 9f5a10bf..0c74189e 100644 --- a/backend/tests/test_clip.py +++ b/backend/tests/test_clip.py @@ -531,6 +531,9 @@ def test_a_youtube_clip_hands_the_range_to_yt_dlp(monkeypatch, tmp_path): def __enter__(self): return self + def add_post_processor(self, pp, when="post_process"): + pass # the egress format filter; nothing here to filter + def __exit__(self, *_a): return False diff --git a/backend/tests/test_egress.py b/backend/tests/test_egress.py new file mode 100644 index 00000000..4ce83b2d --- /dev/null +++ b/backend/tests/test_egress.py @@ -0,0 +1,322 @@ +"""SEC-17: the egress gate judges the address a guarded download actually dials. + +Two loopback addresses stand in for the two sides: 127.0.0.2 plays a public host, 127.0.0.1 our own +network. The gate under test refuses exactly 127.0.0.1, so every assertion is about the judgement and +the wiring, with real sockets, a real yt-dlp and a real ffmpeg — no mocked transport. +""" + +import contextlib +import http.server +import ipaddress +import socket +import socketserver +import threading +from pathlib import Path + +import pytest +import yt_dlp +from yt_dlp.utils import download_range_func + +from app.downloads import adapters, egress, errors, formats + +PUBLIC = "127.0.0.2" +INTERNAL = "127.0.0.1" + + +def _forbid_internal(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool: + return str(ip) == INTERNAL + + +@pytest.fixture +def gate(): + g = egress.start_gate(forbidden=_forbid_internal) + yield g + g.shutdown() + g.server_close() + + +class _Listener: + """A TCP listener that only counts who connected — the internal service under attack.""" + + def __init__(self, host: str = INTERNAL, port: int = 0): + self.hits = 0 + self.sock = socket.socket() + self.sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + self.sock.bind((host, port)) + self.sock.listen() + self.port = self.sock.getsockname()[1] + threading.Thread(target=self._serve, daemon=True).start() + + def _serve(self): + while True: + try: + conn, _ = self.sock.accept() + except OSError: + return + self.hits += 1 + with contextlib.suppress(OSError): + conn.sendall(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok") + conn.close() + + def close(self): + self.sock.close() + + +def _public_server(routes: dict, port: int = 0, directory: Path | None = None): + """An HTTP server on the 'public' address: `routes` maps a path to (status, headers); anything + else is served from `directory`.""" + + class H(http.server.SimpleHTTPRequestHandler): + def __init__(self, *a, **k): + super().__init__(*a, directory=str(directory or "."), **k) + + def log_message(self, *a): + pass + + def do_GET(self): + if self.path in routes: + status, headers, body = routes[self.path] + self.send_response(status) + for k, v in headers.items(): + self.send_header(k, v) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + return + super().do_GET() + + srv = socketserver.ThreadingTCPServer((PUBLIC, port), H) + srv.daemon_threads = True + threading.Thread(target=srv.serve_forever, daemon=True).start() + return srv + + +def _guarded_opts(tmp_path: Path, gate_url: str, **extra) -> dict: + opts = formats.build_ydl_opts({}, str(tmp_path / "%(id)s.%(ext)s"), lambda d: None, egress=True) + opts.update(retries=0, fragment_retries=0, extractor_retries=0, socket_timeout=5) + opts["proxy"] = gate_url # the test's gate (it lets 127.0.0.2 through), not the worker's + opts.update(extra) + return opts + + +def _download_error(opts: dict, url: str) -> str: + with pytest.raises(yt_dlp.utils.DownloadError) as exc, egress.open_ydl(opts) as ydl: + ydl.extract_info(url, download=True) + return str(exc.value) + + +# --- the judgement --------------------------------------------------------------------------------- + + +def test_a_name_with_any_forbidden_answer_is_refused(): + def resolve(answers): + return lambda host, port, *a: [(2, 1, 6, "", (ip, port)) for ip in answers] + + assert egress.vetted_address( + "x", 80, resolve=resolve([PUBLIC]), forbidden=_forbid_internal + ) == ( + PUBLIC, + 80, + ) + assert ( + egress.vetted_address( + "x", 80, resolve=resolve([PUBLIC, INTERNAL]), forbidden=_forbid_internal + ) + is None + ) + + def no_answer(*a): + raise socket.gaierror("no such name") + + assert egress.vetted_address("x", 80, resolve=no_answer, forbidden=_forbid_internal) is None + + +def test_the_real_judgement_refuses_our_own_networks(): + # The worker's gate uses urlguard's judgement unchanged — the same one the enqueue check uses. + for addr in ("127.0.0.1", "10.1.2.3", "172.18.0.5", "192.168.1.1", "169.254.169.254"): + assert egress.vetted_address(addr, 80, forbidden=egress.urlguard.is_forbidden) is None, addr + assert egress.vetted_address("93.184.215.14", 80, forbidden=egress.urlguard.is_forbidden) + + +def test_the_gate_dials_the_address_it_vetted_not_a_second_answer(gate): + # DNS rebinding: a name that answers public for the check and private for the fetch. The gate + # resolves once per connection and dials that answer. + public = _public_server({"/": (200, {}, b"public")}) + internal = _Listener(INTERNAL, public.server_address[1]) + calls = [] + + def rebinding(host, port, *a): + calls.append(host) + ip = PUBLIC if len(calls) == 1 else INTERNAL + return [(2, 1, 6, "", (ip, port))] + + gate.resolve = rebinding + try: + with socket.create_connection(("127.0.0.1", gate.server_address[1])) as s: + s.sendall( + f"GET http://rebind.test:{public.server_address[1]}/ HTTP/1.1\r\n" + f"Host: rebind.test\r\n\r\n".encode() + ) + answer = b"".join(iter(lambda: s.recv(4096), b"")) + assert answer.endswith(b"public") + assert calls == ["rebind.test"] + assert internal.hits == 0 + finally: + public.shutdown() + internal.close() + + +def test_a_tunnel_into_the_network_is_refused_before_it_is_dialled(gate): + internal = _Listener() + try: + with socket.create_connection(("127.0.0.1", gate.server_address[1])) as s: + s.sendall(f"CONNECT {INTERNAL}:{internal.port} HTTP/1.1\r\n\r\n".encode()) + answer = s.recv(4096) + assert answer.startswith(b"HTTP/1.1 403") + assert internal.hits == 0 + finally: + internal.close() + + +# --- a real yt-dlp through it ---------------------------------------------------------------------- + + +def test_a_redirect_into_the_network_reaches_it_unguarded_and_not_through_the_gate(tmp_path, gate): + internal = _Listener() + public = _public_server( + {"/v.mp4": (302, {"Location": f"http://{INTERNAL}:{internal.port}/v.mp4"}, b"")} + ) + url = f"http://{PUBLIC}:{public.server_address[1]}/v.mp4" + try: + # The hole, as it was: yt-dlp follows the 3xx into our network. + bare = {"outtmpl": str(tmp_path / "bare" / "%(id)s.%(ext)s"), "quiet": True, "retries": 0} + with contextlib.suppress(yt_dlp.utils.DownloadError), yt_dlp.YoutubeDL(bare) as ydl: + ydl.extract_info(url, download=True) + assert internal.hits >= 1 + before = internal.hits + + _download_error(_guarded_opts(tmp_path, gate.url), url) + assert internal.hits == before + finally: + public.shutdown() + internal.close() + + +def test_a_live_and_a_dead_internal_port_leave_the_same_text(tmp_path, gate): + live = _Listener() + dead = _Listener() + dead_port = dead.port + dead.close() + public = _public_server( + { + "/live.mp4": (302, {"Location": f"http://{INTERNAL}:{live.port}/x"}, b""), + "/dead.mp4": (302, {"Location": f"http://{INTERNAL}:{dead_port}/x"}, b""), + } + ) + base = f"http://{PUBLIC}:{public.server_address[1]}" + try: + stored = [] + for name in ("live", "dead"): + msg = _download_error(_guarded_opts(tmp_path / name, gate.url), f"{base}/{name}.mp4") + stored.append(errors.user_error_text("url", msg, errors.classify(msg))) + assert stored[0] == stored[1] + assert live.hits == 0 + finally: + public.shutdown() + live.close() + + +def test_a_format_fetched_past_the_gate_is_never_chosen(tmp_path): + # ffmpeg speaks rtmp, rtsp, tcp… directly, where no HTTP proxy is asked. A guarded download keeps + # only the formats fetched over http(s). + opts = _guarded_opts(tmp_path, "http://127.0.0.1:9") + info = { + "id": "x", + "title": "x", + "extractor": "generic", + "extractor_key": "Generic", + "webpage_url": f"http://{PUBLIC}/page", + "formats": [ + {"format_id": "rtmp", "url": f"rtmp://{INTERNAL}/live", "ext": "flv", "height": 1080}, + {"format_id": "web", "url": f"http://{PUBLIC}/a.mp4", "ext": "mp4", "height": 360}, + ], + } + with egress.open_ydl(opts) as ydl: + chosen = ydl.process_ie_result(dict(info), download=False) + assert chosen["format_id"] == "web" + + only_rtmp = dict(info, formats=[info["formats"][0]]) + with ( + pytest.raises(yt_dlp.utils.YoutubeDLError, match="Requested format is not available"), + egress.open_ydl(opts) as ydl, + ): + ydl.process_ie_result(only_rtmp, download=False) + + +def _clip_hits(tmp_path: Path, segment: str, proxy: str) -> int: + """How many connections the internal listener takes while a clip (`download_ranges`, which + yt-dlp hands to ffmpeg even for an HLS format) is cut from a playlist whose one segment is + `segment` — formatted with the listener's address.""" + internal = _Listener() + playlist = ( + "#EXTM3U\n#EXT-X-VERSION:3\n#EXT-X-TARGETDURATION:2\n#EXTINF:2.0,\n" + + segment.format(host=INTERNAL, port=internal.port) + + "\n#EXT-X-ENDLIST\n" + ).encode() + public = _public_server( + {"/p.m3u8": (200, {"Content-Type": "application/vnd.apple.mpegurl"}, playlist)} + ) + try: + opts = _guarded_opts(tmp_path, proxy, download_ranges=download_range_func([], [(0, 1)])) + with contextlib.suppress(yt_dlp.utils.DownloadError), egress.open_ydl(opts) as ydl: + ydl.extract_info(f"http://{PUBLIC}:{public.server_address[1]}/p.m3u8", download=True) + return internal.hits + finally: + public.shutdown() + internal.close() + + +def test_a_clip_cut_by_ffmpeg_reaches_a_segment_only_through_the_gate(tmp_path, gate): + # Without the gate, ffmpeg fetches a hostile playlist's segment from inside our network — the + # control that shows this path is real. Through the gate it is never dialled. + segment = "http://{host}:{port}/s.ts" + assert _clip_hits(tmp_path / "bare", segment, proxy="") >= 1 + assert _clip_hits(tmp_path / "gated", segment, proxy=gate.url) == 0 + + +def test_ffmpeg_itself_refuses_a_raw_tcp_segment(tmp_path): + # A boundary, not our code: the HTTP proxy cannot see a tcp:// segment, and measured, ffmpeg's + # HLS reader does not open one from an http playlist. This fails if an ffmpeg upgrade starts to. + assert _clip_hits(tmp_path, "tcp://{host}:{port}/", proxy="") == 0 + + +# --- the wiring ------------------------------------------------------------------------------------ + + +def test_only_youtube_downloads_skip_the_gate(): + assert adapters.SourceAdapter.egress_guarded is True + assert adapters.resolve_adapter("url", "https://www.facebook.com/x").egress_guarded is True + assert adapters.resolve_adapter("url", "https://vimeo.com/1").egress_guarded is True + assert adapters.resolve_adapter("youtube", "dQw4w9WgXcQ").egress_guarded is False + + +def test_a_guarded_download_goes_through_the_gate_whatever_its_adapter_adds(): + opts = formats.build_ydl_opts( + {}, "/tmp/x", lambda d: None, extra={"proxy": "", "hls_prefer_native": False}, egress=True + ) + assert opts["proxy"] == egress.proxy_url() + assert opts["hls_prefer_native"] is True + assert opts[egress.GUARDED] is True + assert "proxy" not in formats.build_ydl_opts({}, "/tmp/x", lambda d: None, egress=False) + + +def test_every_yt_dlp_this_app_opens_goes_through_open_ydl(): + app = Path(egress.__file__).resolve().parents[1] + offenders = [ + f"{p.relative_to(app)}:{n}" + for p in sorted(app.rglob("*.py")) + if p.name != "egress.py" + for n, line in enumerate(p.read_text(encoding="utf-8").splitlines(), 1) + if "YoutubeDL(" in line + ] + assert offenders == [] diff --git a/backend/tests/test_facebook_impersonation.py b/backend/tests/test_facebook_impersonation.py index fc5c7840..61cfb150 100644 --- a/backend/tests/test_facebook_impersonation.py +++ b/backend/tests/test_facebook_impersonation.py @@ -73,6 +73,9 @@ def ydl(monkeypatch): def __enter__(self): return self + def add_post_processor(self, pp, when="post_process"): + pass # the egress format filter; nothing here to filter + def __exit__(self, *exc): return False diff --git a/backend/tests/test_facebook_titles.py b/backend/tests/test_facebook_titles.py index 25372183..145eb320 100644 --- a/backend/tests/test_facebook_titles.py +++ b/backend/tests/test_facebook_titles.py @@ -36,6 +36,9 @@ def _fake_ydl(monkeypatch, outcome): def __enter__(self): return self + def add_post_processor(self, pp, when="post_process"): + pass # the egress format filter; nothing here to filter + def __exit__(self, *exc): return False @@ -43,7 +46,7 @@ def _fake_ydl(monkeypatch, outcome): seen.append({**self.opts, "url": url}) return outcome - monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", FakeYDL) + monkeypatch.setattr("yt_dlp.YoutubeDL", FakeYDL) return seen diff --git a/backend/tests/test_failure_lines.py b/backend/tests/test_failure_lines.py index e5122140..de021f3a 100644 --- a/backend/tests/test_failure_lines.py +++ b/backend/tests/test_failure_lines.py @@ -359,6 +359,9 @@ def test_the_metadata_backfill_names_the_asset_and_not_the_download(sink, monkey def __enter__(self): return self + def add_post_processor(self, pp, when="post_process"): + pass # the egress format filter; nothing here to filter + def __exit__(self, *exc): return False @@ -367,7 +370,7 @@ def test_the_metadata_backfill_names_the_asset_and_not_the_download(sink, monkey f"ERROR: [youtube] dQw4w9WgXcQ: Video unavailable ({target})" ) - monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", _Refusing) + monkeypatch.setattr("yt_dlp.YoutubeDL", _Refusing) assert backfill.extract_meta(url, adapters.YouTubeAdapter(), 11) is None lines = [o for o in sink.objects() if "meta extract failed" in o["msg"]] @@ -386,13 +389,16 @@ def test_the_metadata_backfill_takes_out_a_generic_name_whatever_it_holds(sink, def __enter__(self): return self + def add_post_processor(self, pp, when="post_process"): + pass # the egress format filter; nothing here to filter + def __exit__(self, *exc): return False def extract_info(self, target, download): raise yt_dlp.utils.DownloadError("ERROR: [generic] Our Trip: Day 1: Unable to download") - monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", _Refusing) + monkeypatch.setattr("yt_dlp.YoutubeDL", _Refusing) assert backfill.extract_meta(url, adapters.SourceAdapter(), 11) is None lines = [o for o in sink.objects() if "meta extract failed" in o["msg"]] @@ -449,6 +455,9 @@ def test_a_retried_attempt_names_the_host_and_not_the_video(sink, monkeypatch, t def __enter__(self): return self + def add_post_processor(self, pp, when="post_process"): + pass # the egress format filter; nothing here to filter + def __exit__(self, *exc): return False @@ -463,6 +472,7 @@ def test_a_retried_attempt_names_the_host_and_not_the_video(sink, monkeypatch, t resolve_url=lambda target, opts: target, ydl_opts_extra=lambda clients: {}, retriable=lambda text: True, + egress_guarded=False, ) monkeypatch.setattr(pipeline.yt_dlp, "YoutubeDL", _YDL) monkeypatch.setattr(pipeline, "_touch_job", lambda job_id: None) diff --git a/backend/tests/test_formats.py b/backend/tests/test_formats.py index a0369c5d..d6ef2c61 100644 --- a/backend/tests/test_formats.py +++ b/backend/tests/test_formats.py @@ -15,16 +15,20 @@ from app.downloads import formats def _opts(spec): - return formats.build_ydl_opts(spec, "/tmp/%(id)s.%(ext)s", lambda d: None) + return formats.build_ydl_opts(spec, "/tmp/%(id)s.%(ext)s", lambda d: None, egress=False) def test_adapter_extra_opts_are_merged_in(): # The SourceAdapter contributes source-specific opts (e.g. YouTube's extractor_args) via `extra`. extra = {"extractor_args": {"youtube": {"player_client": ["web"]}}} - opts = formats.build_ydl_opts({}, "/tmp/%(id)s.%(ext)s", lambda d: None, extra=extra) + opts = formats.build_ydl_opts( + {}, "/tmp/%(id)s.%(ext)s", lambda d: None, extra=extra, egress=False + ) assert opts["extractor_args"]["youtube"]["player_client"] == ["web"] # No extra → no extractor_args (a generic source contributes nothing). - assert "extractor_args" not in formats.build_ydl_opts({}, "/tmp/x", lambda d: None) + assert "extractor_args" not in formats.build_ydl_opts( + {}, "/tmp/x", lambda d: None, egress=False + ) def test_mp4_default_selects_avc1_with_progressive_fallback(): @@ -113,7 +117,7 @@ def test_a_playlist_url_can_only_yield_one_item(): def test_the_remaining_quota_becomes_a_hard_size_ceiling(): - opts = formats.build_ydl_opts({}, "/tmp/x", lambda d: None, max_bytes=5_000_000) + opts = formats.build_ydl_opts({}, "/tmp/x", lambda d: None, max_bytes=5_000_000, egress=False) assert opts["max_filesize"] == 5_000_000 @@ -122,7 +126,7 @@ def test_no_budget_means_no_size_filter(): # enqueue, not an impossible yt-dlp filter to express here. for budget in (None, 0, -1): assert "max_filesize" not in formats.build_ydl_opts( - {}, "/tmp/x", lambda d: None, max_bytes=budget + {}, "/tmp/x", lambda d: None, max_bytes=budget, egress=False ) @@ -130,7 +134,12 @@ def test_a_source_adapter_cannot_raise_the_size_ceiling(): # The adapter merge happens after max_filesize is set, so this pins that no adapter option can # widen a storage cap — the mirror of the cookiefile rule, in the opposite direction. opts = formats.build_ydl_opts( - {}, "/tmp/x", lambda d: None, extra={"max_filesize": 999_999_999}, max_bytes=5_000_000 + {}, + "/tmp/x", + lambda d: None, + extra={"max_filesize": 999_999_999}, + max_bytes=5_000_000, + egress=False, ) assert opts["max_filesize"] == 5_000_000 @@ -197,7 +206,7 @@ def test_yt_dlp_refuses_a_file_over_the_cap_without_raising_and_the_watch_sees_i for cap, refused in ((1_000, True), (10_000_000, False)): out = tmp_path / f"out-{cap}" opts = formats.build_ydl_opts( - {}, str(out / "%(id)s.%(ext)s"), lambda d: None, max_bytes=cap + {}, str(out / "%(id)s.%(ext)s"), lambda d: None, max_bytes=cap, egress=False ) with formats.watching_size_cap() as over, yt_dlp.YoutubeDL(opts) as ydl: ydl.extract_info(url, download=True) diff --git a/backend/tests/test_ytdlp_logging.py b/backend/tests/test_ytdlp_logging.py index bc98cacc..57ff10f2 100644 --- a/backend/tests/test_ytdlp_logging.py +++ b/backend/tests/test_ytdlp_logging.py @@ -216,7 +216,7 @@ def test_a_line_with_no_url_is_left_alone(sink): def test_the_download_options_carry_the_logger(): - opts = formats.build_ydl_opts({}, "%(id)s.%(ext)s", lambda d: None) + opts = formats.build_ydl_opts({}, "%(id)s.%(ext)s", lambda d: None, egress=False) assert opts["logger"] is formats._YDL_LOG @@ -227,7 +227,7 @@ def test_a_source_adapter_cannot_silence_it(): adapter must not be able to take away the one account of what went wrong that a failed job leaves behind.""" opts = formats.build_ydl_opts( - {}, "%(id)s.%(ext)s", lambda d: None, extra={"logger": None, "quiet": False} + {}, "%(id)s.%(ext)s", lambda d: None, extra={"logger": None, "quiet": False}, egress=False ) assert opts["logger"] is formats._YDL_LOG From 1bf3d6a592d9b0a02e340d1480393065e2d9d24e Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 6 Nov 2026 05:23:02 +0100 Subject: [PATCH 03/23] fix(downloads): a download that hangs up mid-transfer is not an error at the egress gate Seen live: yt-dlp closes a connection once it has read what it needed, and the gate printed a traceback for each. A reset ends the tunnel quietly; anything else the gate cannot finish is a log line with its code, not bare stderr. --- backend/app/downloads/egress.py | 14 ++++++++++++++ backend/tests/test_egress.py | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/backend/app/downloads/egress.py b/backend/app/downloads/egress.py index 991e8ce8..a5649c06 100644 --- a/backend/app/downloads/egress.py +++ b/backend/app/downloads/egress.py @@ -93,6 +93,15 @@ def _read_head(sock: socket.socket) -> bytes | None: def _pipe(a: socket.socket, b: socket.socket) -> None: + try: + _relay(a, b) + except OSError: + # silent-ok: one side hung up mid-transfer (yt-dlp closes a connection once it has read a + # file's size, or gives up on a slow host); whichever side cared reports it itself. + return + + +def _relay(a: socket.socket, b: socket.socket) -> None: socks = [a, b] while True: ready, _, _ = select.select(socks, [], [], _IDLE_S) @@ -196,6 +205,11 @@ class _Gate(socketserver.ThreadingTCPServer): def url(self) -> str: return f"http://127.0.0.1:{self.server_address[1]}" + def handle_error(self, request, client_address) -> None: + # socketserver's default prints a bare traceback to stderr, outside the log; a connection the + # gate could not finish is a line with its code instead. + log.exception("egress gate dropped a connection", extra=logs.event("download.egress.error")) + def start_gate( *, resolve: Resolver = socket.getaddrinfo, forbidden: Forbidden = urlguard.is_forbidden diff --git a/backend/tests/test_egress.py b/backend/tests/test_egress.py index 4ce83b2d..012dd3d1 100644 --- a/backend/tests/test_egress.py +++ b/backend/tests/test_egress.py @@ -10,7 +10,9 @@ import http.server import ipaddress import socket import socketserver +import struct import threading +import time from pathlib import Path import pytest @@ -178,6 +180,24 @@ def test_a_tunnel_into_the_network_is_refused_before_it_is_dialled(gate): internal.close() +def test_a_client_that_hangs_up_mid_transfer_is_not_an_error(gate, monkeypatch): + # yt-dlp closes a connection once it has read what it needed; seen live as a traceback the gate + # printed for every such download. + errors_seen = [] + monkeypatch.setattr(gate, "handle_error", lambda *a: errors_seen.append(a)) + public = _public_server({"/big": (200, {}, b"x" * 4_000_000)}) + try: + s = socket.create_connection(("127.0.0.1", gate.server_address[1])) + s.sendall(f"GET http://{PUBLIC}:{public.server_address[1]}/big HTTP/1.1\r\n\r\n".encode()) + s.recv(1024) + s.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack("ii", 1, 0)) + s.close() # a reset, not a polite close + time.sleep(0.5) + assert errors_seen == [] + finally: + public.shutdown() + + # --- a real yt-dlp through it ---------------------------------------------------------------------- From 5b846136adca714b78c2c89dc6445c30a18e6f9d Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 7 Nov 2026 02:20:03 +0100 Subject: [PATCH 04/23] fix(catalog): a 24/7 stream archive's years-long length is unknown, not summed or sorted by YouTube reports a 24/7 stream's archive with the years it has run (prod: 122,962,914 s), and one such video made the Watch later total read 44,073 hours. A length past a day is now no length (`Video.length_seconds`, LENGTH_MAX_S): the API sends none, the playlist and channel totals leave it out and count it as `live_without_length` ("+N live" beside the sum), and the duration filter, sort and Home budget never match it. Migration 0117 moves the duration sort indexes onto the length expression. An AST test holds who may still read the raw column. The migration 0110 test now ends its read before the way down: 0117 builds an index CONCURRENTLY there, which waited for that open transaction forever. --- .../0117_videos_length_sort_indexes.py | 76 ++++++++ backend/app/catalog/models.py | 51 ++++- backend/app/catalog/queries.py | 13 +- backend/app/downloads/service.py | 4 +- backend/app/feedquery.py | 12 +- backend/app/reader/queries.py | 32 +-- backend/app/routes/channels.py | 4 +- backend/app/routes/feed.py | 7 +- backend/app/routes/playlists.py | 12 +- backend/tests/test_migration_0110.py | 3 + backend/tests/test_reader_service.py | 4 +- backend/tests/test_video_length.py | 182 ++++++++++++++++++ .../features/video/liveWithoutLength.test.ts | 21 ++ .../src/features/video/liveWithoutLength.ts | 18 ++ frontend/src/i18n/locales/en/card.json | 2 + frontend/src/i18n/locales/hu/card.json | 2 + frontend/src/lib/api/channels.ts | 2 + frontend/src/lib/api/playlists.ts | 2 + .../channels/channelColumns.dom.test.tsx | 1 + .../src/modules/channels/channelColumns.tsx | 5 + .../channels/useChannelActions.dom.test.tsx | 1 + .../modules/playlists/Playlists.dom.test.tsx | 12 ++ frontend/src/modules/playlists/Playlists.tsx | 8 +- .../playlists/PlaylistsRail.drop.dom.test.tsx | 1 + .../PlaylistsRail.rename.dom.test.tsx | 1 + .../src/modules/playlists/PlaylistsRail.tsx | 9 +- 26 files changed, 446 insertions(+), 39 deletions(-) create mode 100644 backend/alembic/versions/0117_videos_length_sort_indexes.py create mode 100644 backend/tests/test_video_length.py create mode 100644 frontend/src/features/video/liveWithoutLength.test.ts create mode 100644 frontend/src/features/video/liveWithoutLength.ts diff --git a/backend/alembic/versions/0117_videos_length_sort_indexes.py b/backend/alembic/versions/0117_videos_length_sort_indexes.py new file mode 100644 index 00000000..7d1f1e30 --- /dev/null +++ b/backend/alembic/versions/0117_videos_length_sort_indexes.py @@ -0,0 +1,76 @@ +"""videos: the duration sorts ride an index on the video's length, not on the raw column + +Revision ID: 0117_videos_length_sort_indexes +Revises: 0116_followed_explored_channels +Create Date: 2026-11-07 + +A YouTube length past a day is not a length (`catalog.models.LENGTH_MAX_S`): a 24/7 stream's archive +reports the years it has run. The feed's duration sorts now order by `Video.length_seconds`, which +is NULL for such a video, so "longest first" no longer opens on a stream archive. Migration 0096's +two duration indexes are on the raw column and no longer match that ORDER BY; they are replaced by +the same indexes over the length expression. The expression is spelled out here as SQLAlchemy emits +it, with the limit as a literal (an index matches a query's constant, never a bound parameter); +tests/test_feed_sort_indexes.py fails if the two drift apart. + +CONCURRENTLY (autocommit_block), re-runnable, as in 0096. +""" + +from alembic import op +from sqlalchemy import text + +revision: str = "0117_videos_length_sort_indexes" +down_revision: str | None = "0116_followed_explored_channels" +branch_labels: str | None = None +depends_on: str | None = None + +# Frozen with this migration: `catalog.models.LENGTH_MAX_S` is 24 h. +LENGTH = "(CASE WHEN (duration_seconds > 86400) THEN NULL ELSE duration_seconds END)" + +NEW = { + "ix_videos_length_desc": f"{LENGTH} DESC NULLS LAST, id", + "ix_videos_length_asc": f"{LENGTH} ASC NULLS LAST, id", +} +OLD = { + "ix_videos_duration_desc": "duration_seconds DESC NULLS LAST, id", + "ix_videos_duration_asc": "duration_seconds ASC NULLS LAST, id", +} + + +def _valid(name: str) -> bool | None: + """The index's `indisvalid`, or None when there is no index of that name.""" + row = ( + op.get_bind() + .execute( + text( + "SELECT i.indisvalid FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid " + "WHERE c.relname = :name" + ), + {"name": name}, + ) + .first() + ) + return None if row is None else bool(row[0]) + + +def _build(name: str, columns: str) -> None: + """CREATE INDEX CONCURRENTLY, re-runnable: an INVALID leftover of a failed build is dropped first + (see 0096 `_build`).""" + if _valid(name) is False: + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name}") + op.execute(f"CREATE INDEX CONCURRENTLY IF NOT EXISTS {name} ON videos ({columns})") + + +def upgrade() -> None: + with op.get_context().autocommit_block(): + for name, columns in NEW.items(): + _build(name, columns) + for name in OLD: + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name}") + + +def downgrade() -> None: + with op.get_context().autocommit_block(): + for name, columns in OLD.items(): + _build(name, columns) + for name in NEW: + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name}") diff --git a/backend/app/catalog/models.py b/backend/app/catalog/models.py index 5ea760b9..a55a9e84 100644 --- a/backend/app/catalog/models.py +++ b/backend/app/catalog/models.py @@ -16,10 +16,16 @@ from sqlalchemy import ( String, Text, UniqueConstraint, + and_, + case, func, + literal_column, + null, ) from sqlalchemy.dialects.postgresql import TSVECTOR +from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import Mapped, mapped_column, relationship +from sqlalchemy.sql.elements import ColumnElement from app.db import Base from app.models import TimestampMixin @@ -114,6 +120,20 @@ class Subscription(Base, TimestampMixin): # hidden from feeds/search and revisited by the refresh job until they settle into a VOD. LIVE_OR_UPCOMING = ("live", "upcoming") +# A YouTube length past a day is not a length. A 24/7 stream's archive (`was_live`) reports the years +# the stream has run (measured on prod, 2026-11-06: 122,962,914 s), and one such video made a Watch +# later total read 44,073 hours. YouTube caps an upload at 12 hours, so nothing real is lost. Such a +# length is unknown wherever a length is used: the API sends none (`Video.length_seconds`), sums leave +# it out and count it as `live_without_length`, and the duration filter, sort and Home budget never +# match it. The raw `duration_seconds` stays for what it does prove — that the video is not a Short — +# and for the jobs that ask whether YouTube has answered yet (test_video_length holds who reads it). +LENGTH_MAX_S = 24 * 3600 + + +def known_length(seconds: int | None) -> int | None: + """`seconds` as a video's length, or None when it is not one (see LENGTH_MAX_S).""" + return seconds if seconds is not None and seconds <= LENGTH_MAX_S else None + class Video(Base, TimestampMixin): """A YouTube video. Shared across users; per-user state lives elsewhere.""" @@ -141,8 +161,8 @@ class Video(Base, TimestampMixin): ) thumbnail_url: Mapped[str | None] = mapped_column(String(1024)) # The feed's views / duration / title sorts each ride a migration-defined index in their exact - # ORDER BY, one per direction (and per ICU collation for title): migration 0096, held to - # `feedquery.order_by` by test_feed_sort_indexes. + # ORDER BY, one per direction (and per ICU collation for title): migration 0096 (the duration + # pair since 0117, on `length_seconds`), held to `feedquery.order_by` by test_feed_sort_indexes. duration_seconds: Mapped[int | None] = mapped_column(Integer) view_count: Mapped[int | None] = mapped_column(BigInteger) like_count: Mapped[int | None] = mapped_column(BigInteger) @@ -198,6 +218,33 @@ class Video(Base, TimestampMixin): unavailable_since: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) unavailable_reason: Mapped[str | None] = mapped_column(String(24)) + @hybrid_property + def length_seconds(self) -> int | None: + """The video's length, or None when it is unknown — not fetched yet, or not a length at all + (`LENGTH_MAX_S`). What every reader of a length reads.""" + return known_length(self.duration_seconds) + + @length_seconds.inplace.expression + @classmethod + def _length_seconds_expression(cls) -> ColumnElement[int | None]: + # The limit is a literal, not a bound parameter: the duration sort indexes (migration 0117) + # are built on this very expression, and Postgres matches an index to a query's constant only. + return case( + (cls.duration_seconds > literal_column(str(LENGTH_MAX_S)), null()), + else_=cls.duration_seconds, + ) + + @hybrid_property + def live_without_length(self) -> bool: + """A live video (on air, upcoming, a premiere or an archive) whose length is unknown: what a + total leaves out and names as "+N live".""" + return self.live_status != "none" and self.length_seconds is None + + @live_without_length.inplace.expression + @classmethod + def _live_without_length_expression(cls) -> ColumnElement[bool]: + return and_(cls.live_status != "none", cls.length_seconds.is_(None)) + # Weighted full-text search document (DB-generated, never written by the app): title (A) > # creator keywords + search queries (B) > truncated description (C). ts_rank honours the # weights so title matches outrank description matches. Backed by a GIN index (migration diff --git a/backend/app/catalog/queries.py b/backend/app/catalog/queries.py index 63b2caee..1955f458 100644 --- a/backend/app/catalog/queries.py +++ b/backend/app/catalog/queries.py @@ -44,7 +44,8 @@ def channel_titles(db: Session, ids: set[str]) -> dict[str, str]: def channel_aggregates(channel_ids: list[str]) -> Select: - """Per-channel stored count, last upload and total duration, in one grouped pass. Every column it + """Per-channel stored count, last upload, total duration and the live videos the total cannot + hold (`Video.live_without_length`), in one grouped pass. Every column it reads is in `ix_videos_channel_pub_cover` (migration 0095), so Postgres answers it from the index alone; a test holds that (test_channel_list_index_only).""" return ( @@ -52,15 +53,19 @@ def channel_aggregates(channel_ids: list[str]) -> Select: Video.channel_id, func.count(Video.id), func.max(Video.published_at), - func.coalesce(func.sum(Video.duration_seconds), 0), + func.coalesce(func.sum(Video.length_seconds), 0), + func.count().filter(Video.live_without_length), ) .where(Video.channel_id.in_(channel_ids)) .group_by(Video.channel_id) ) -def aggregates(db: Session, channel_ids: list[str]) -> list[tuple[str, int, datetime | None, int]]: - """`channel_aggregates`, run: (channel id, stored videos, last upload, total seconds).""" +def aggregates( + db: Session, channel_ids: list[str] +) -> list[tuple[str, int, datetime | None, int, int]]: + """`channel_aggregates`, run: (channel id, stored videos, last upload, total seconds, live + videos without a length).""" if not channel_ids: return [] return list(db.execute(channel_aggregates(channel_ids)).tuples()) diff --git a/backend/app/downloads/service.py b/backend/app/downloads/service.py index e2e80910..97073d99 100644 --- a/backend/app/downloads/service.py +++ b/backend/app/downloads/service.py @@ -153,7 +153,7 @@ def populate_from_catalog(db: Session, asset: MediaAsset) -> bool: return False asset.title = video.title asset.thumbnail_url = video.thumbnail_url - asset.duration_s = video.duration_seconds + asset.duration_s = video.length_seconds if video.published_at: asset.upload_date = video.published_at.date() channel = db.get(Channel, video.channel_id) @@ -337,7 +337,7 @@ def _clip_source_facts(db: Session, spec: dict) -> dict: "title": video.title, "uploader": channel.title if channel else None, "thumbnail_url": video.thumbnail_url, - "length_s": video.duration_seconds, + "length_s": video.length_seconds, } item = plex_lookups.by_rating_key(db, PlexItem, spec["ref"]) if item is None: diff --git a/backend/app/feedquery.py b/backend/app/feedquery.py index c703846c..173c806e 100644 --- a/backend/app/feedquery.py +++ b/backend/app/feedquery.py @@ -68,7 +68,7 @@ def feed_columns(user: User, state) -> list: Channel.handle.label("channel_handle"), Video.published_at, Video.thumbnail_url, - Video.duration_seconds, + Video.length_seconds.label("length_seconds"), Video.view_count, Video.is_short, Video.live_status, @@ -88,7 +88,7 @@ def serialize(row) -> dict: "channel_url": _channel_url(row.channel_id, row.channel_handle), "published_at": iso(row.published_at), "thumbnail_url": row.thumbnail_url, - "duration_seconds": row.duration_seconds, + "duration_seconds": row.length_seconds, "view_count": row.view_count, "is_short": row.is_short, "live_status": row.live_status, @@ -260,9 +260,9 @@ def filtered_query( # OR across the picked channels; every other filter still applies on top. query = query.where(Video.channel_id.in_(channel_ids)) if min_duration is not None: - query = query.where(Video.duration_seconds >= min_duration) + query = query.where(Video.length_seconds >= min_duration) if max_duration is not None: - query = query.where(Video.duration_seconds <= max_duration) + query = query.where(Video.length_seconds <= max_duration) if max_age_days is not None: cutoff = datetime.now(timezone.utc).timestamp() - max_age_days * 86400 query = query.where(Video.published_at >= datetime.fromtimestamp(cutoff, tz=timezone.utc)) @@ -503,10 +503,10 @@ def sort_keys(sort: str, seed: int, rank_expr=None, lang: str | None = None) -> "views": [{"expr": Video.view_count, "asc": False, "nullable": True, "kind": "num"}], "views_asc": [{"expr": Video.view_count, "asc": True, "nullable": True, "kind": "num"}], "duration_desc": [ - {"expr": Video.duration_seconds, "asc": False, "nullable": True, "kind": "num"} + {"expr": Video.length_seconds, "asc": False, "nullable": True, "kind": "num"} ], "duration_asc": [ - {"expr": Video.duration_seconds, "asc": True, "nullable": True, "kind": "num"} + {"expr": Video.length_seconds, "asc": True, "nullable": True, "kind": "num"} ], "title": [{"expr": title, "asc": True, "nullable": True, "kind": "str"}], "title_desc": [{"expr": title, "asc": False, "nullable": True, "kind": "str"}], diff --git a/backend/app/reader/queries.py b/backend/app/reader/queries.py index b5e9c026..2bf2f4d7 100644 --- a/backend/app/reader/queries.py +++ b/backend/app/reader/queries.py @@ -146,12 +146,17 @@ def item_counts(db: Session, ids: list[int]) -> dict[int, int]: ) -def durations(db: Session, ids: list[int]) -> dict[int, int]: - """Each playlist's running time: the sum of its videos' lengths.""" +def durations(db: Session, ids: list[int]) -> dict[int, tuple[int, int]]: + """Each playlist's running time — the sum of its videos' known lengths — and how many live videos + it cannot hold (`Video.live_without_length`).""" return { - pid: int(total or 0) - for pid, total in db.execute( - select(PlaylistItem.playlist_id, func.coalesce(func.sum(Video.duration_seconds), 0)) + pid: (int(total or 0), int(live or 0)) + for pid, total, live in db.execute( + select( + PlaylistItem.playlist_id, + func.coalesce(func.sum(Video.length_seconds), 0), + func.count().filter(Video.live_without_length), + ) .join(Video, Video.id == PlaylistItem.video_id) .where(PlaylistItem.playlist_id.in_(ids)) .group_by(PlaylistItem.playlist_id) @@ -191,11 +196,16 @@ def covers(db: Session, ids: list[int]) -> dict[int, str | None]: ) -def playlist_totals(db: Session, playlist_id: int) -> tuple[int, int, str | None]: - """One playlist's item count, running time and cover. An outer join, so an empty list still - answers (0, 0, None).""" - count, total = db.execute( - select(func.count(PlaylistItem.id), func.coalesce(func.sum(Video.duration_seconds), 0)) +def playlist_totals(db: Session, playlist_id: int) -> tuple[int, int, int, str | None]: + """One playlist's item count, running time, live videos the running time cannot hold + (`Video.live_without_length`) and cover. An outer join, so an empty list still answers + (0, 0, 0, None).""" + count, total, live = db.execute( + select( + func.count(PlaylistItem.id), + func.coalesce(func.sum(Video.length_seconds), 0), + func.count().filter(Video.live_without_length), + ) .select_from(PlaylistItem) .outerjoin(Video, Video.id == PlaylistItem.video_id) .where(PlaylistItem.playlist_id == playlist_id) @@ -207,7 +217,7 @@ def playlist_totals(db: Session, playlist_id: int) -> tuple[int, int, str | None .order_by(PlaylistItem.position) .limit(1) ) - return count or 0, total or 0, cover + return count or 0, total or 0, live or 0, cover def playlist_rows(db: Session, user: User, playlist_id: int) -> list: diff --git a/backend/app/routes/channels.py b/backend/app/routes/channels.py index beedfa9e..33e80a47 100644 --- a/backend/app/routes/channels.py +++ b/backend/app/routes/channels.py @@ -46,11 +46,12 @@ def list_channels(user: User = Depends(current_user), db: Session = Depends(get_ # computed here — the manager table dropped its N/S/L column.) channel_ids = [c.id for _, c in rows] agg: dict[str, dict] = {} - for cid, n, last_up, total_dur in queries.aggregates(db, channel_ids): + for cid, n, last_up, total_dur, live in queries.aggregates(db, channel_ids): agg[cid] = { "stored": n, "last_video_at": iso(last_up), "total_duration_seconds": int(total_dur or 0), + "live_without_length": live, } # Channels already in the *global* deep-backfill queue — i.e. at least one user (any @@ -68,6 +69,7 @@ def list_channels(user: User = Depends(current_user), db: Session = Depends(get_ "stored_videos": (agg.get(ch.id) or {}).get("stored", 0), "last_video_at": (agg.get(ch.id) or {}).get("last_video_at"), "total_duration_seconds": (agg.get(ch.id) or {}).get("total_duration_seconds", 0), + "live_without_length": (agg.get(ch.id) or {}).get("live_without_length", 0), "priority": sub.priority, "hidden": sub.hidden, "deep_requested": sub.deep_requested, diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index 43581896..237cd8f7 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -10,6 +10,7 @@ from sqlalchemy.orm.exc import StaleDataError from app import jobruns, quota, scheduler from app.catalog import queries as catalog_queries, service as catalog_service +from app.catalog.models import known_length from app.auth import current_user from app.db import get_db, release_read_connection from app.mailtext import lang_for_user @@ -287,7 +288,7 @@ def get_video_detail( "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, + "duration_seconds": v.length_seconds, } # The DB path above is free; the off-catalog YouTube lookup below spends a quota unit. @@ -315,8 +316,8 @@ def get_video_detail( "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") + "duration_seconds": known_length( + parse_iso8601_duration(items[0].get("contentDetails", {}).get("duration")) ), } diff --git a/backend/app/routes/playlists.py b/backend/app/routes/playlists.py index 79ac345a..c2a1fe8b 100644 --- a/backend/app/routes/playlists.py +++ b/backend/app/routes/playlists.py @@ -95,10 +95,13 @@ def _video_or_404(db: Session, video_id: str) -> None: def _summary( pl: "Playlist", count: int, - total_duration: int = 0, + duration: tuple[int, int] = (0, 0), has_video: bool | None = None, cover: str | None = None, ) -> dict: + """`duration` is `queries.durations`'s pair: the known running time, and the live videos it + cannot hold — the "+N live" beside it.""" + total_duration, live = duration out = { "id": pl.id, "name": pl.name, @@ -108,6 +111,7 @@ def _summary( "dirty": pl.dirty, "item_count": count, "total_duration_seconds": total_duration, + "live_without_length": live, "cover_thumbnail": cover, # When this list last CHANGED (rename, add/remove/move/reorder) — the rail's # "recently changed first" sort reads it. See `service.touch` for why it isn't automatic. @@ -138,7 +142,7 @@ def list_playlists( _summary( p, counts.get(p.id, 0), - durations.get(p.id, 0), + durations.get(p.id, (0, 0)), (p.id in member) if contains else None, covers.get(p.id), ) @@ -363,8 +367,8 @@ def rename_playlist( raise _refused(400, "name_required") service.rename_playlist(db, pl, name) db.commit() - count, total, cover = queries.playlist_totals(db, pl.id) - return _summary(pl, count, total, cover=cover) + count, total, live, cover = queries.playlist_totals(db, pl.id) + return _summary(pl, count, (total, live), cover=cover) @router.delete("/{playlist_id}") diff --git a/backend/tests/test_migration_0110.py b/backend/tests/test_migration_0110.py index ba3677b8..82fe996a 100644 --- a/backend/tests/test_migration_0110.py +++ b/backend/tests/test_migration_0110.py @@ -45,6 +45,9 @@ def test_an_interval_override_moves_into_sysconfig_and_back(db): } assert conn.execute(text("SELECT to_regclass('scheduler_settings')")).scalar() is None assert scheduler.load_intervals(db)["rss_poll"] == 25 + # The way down rebuilds an index CONCURRENTLY (0117), which waits for every open transaction — + # the read above left this session's open. + db.rollback() command.downgrade(cfg, "0109_job_runs") try: diff --git a/backend/tests/test_reader_service.py b/backend/tests/test_reader_service.py index 647c6825..7d54e648 100644 --- a/backend/tests/test_reader_service.py +++ b/backend/tests/test_reader_service.py @@ -252,7 +252,7 @@ def test_an_add_is_idempotent_and_lands_where_asked(db, make_user): db.commit() assert _ids(db, pl) == ["c", "b"] assert queries.item_counts(db, [pl.id]) == {pl.id: 2} - assert queries.durations(db, [pl.id]) == {pl.id: 120} + assert queries.durations(db, [pl.id]) == {pl.id: (120, 0)} assert queries.containing(db, [pl.id], "b") == {pl.id} @@ -294,7 +294,7 @@ def test_a_rename_and_a_delete(db, make_user): service.add_item(db, pl, "a", mark_dirty=False) service.rename_playlist(db, pl, "New") db.commit() - assert queries.playlist_totals(db, pl.id) == (1, 60, None) + assert queries.playlist_totals(db, pl.id) == (1, 60, 0, None) assert pl.name == "New" service.delete_playlist(db, pl) db.commit() diff --git a/backend/tests/test_video_length.py b/backend/tests/test_video_length.py new file mode 100644 index 00000000..85ecebc5 --- /dev/null +++ b/backend/tests/test_video_length.py @@ -0,0 +1,182 @@ +"""A YouTube length past a day is unknown, everywhere a length is used (`catalog.models.LENGTH_MAX_S`). + +Measured on prod (2026-11-06): two 24/7 streams' archives report 122,962,914 s and 34,762,459 s, and the +owner's Watch later total read "44,073 hr 55 min". Such a video now has no length on any API answer, the +totals leave it out and count it as `live_without_length`, and the duration filter and sort pass it over +like a video whose length is not fetched yet. An AST check holds who may still read the raw column. +DB lane (the AST check needs none). +""" + +import ast +from datetime import datetime, timezone +from pathlib import Path + +from sqlalchemy.dialects import postgresql + +from app.catalog.models import LENGTH_MAX_S, Channel, Subscription, Video, known_length +from app.reader.models import Playlist, PlaylistItem + +YEARS = 122_962_914 # the measured archive +HOUR = 3600 + + +def _seed(db, user): + db.add(Channel(id="UClen", title="Len")) + db.flush() + db.add(Subscription(user_id=user.id, channel_id="UClen", priority=0, hidden=False)) + rows = [ + ("short", 600, "none"), + ("long", 2 * HOUR, "none"), + ("day", LENGTH_MAX_S, "was_live"), # exactly a day still is a length + ("archive", YEARS, "was_live"), + ("onair", None, "live"), + ("unfetched", None, "none"), # not live: not counted as "+N live" + ] + for i, (vid, seconds, live) in enumerate(rows): + db.add( + Video( + id=vid, + channel_id="UClen", + published_at=datetime(2024, 1, i + 1, tzinfo=timezone.utc), + title=vid, + duration_seconds=seconds, + live_status=live, + is_short=False, + via_search=False, + via_explore=False, + ) + ) + pl = Playlist(user_id=user.id, name="Later") + db.add(pl) + db.flush() + for i, (vid, _s, _l) in enumerate(rows): + db.add(PlaylistItem(playlist_id=pl.id, video_id=vid, position=i)) + db.commit() + return pl + + +KNOWN_SUM = 600 + 2 * HOUR + LENGTH_MAX_S + + +def test_the_rule(): + assert known_length(None) is None + assert known_length(LENGTH_MAX_S) == LENGTH_MAX_S + assert known_length(LENGTH_MAX_S + 1) is None + assert known_length(YEARS) is None + + +def test_the_sql_length_carries_its_limit_as_a_literal(): + # The duration sort indexes (migration 0117) are built on this expression with 86400 in it. A + # bound parameter matches no index once the driver reuses a prepared, generic plan, and the + # sort-index test cannot see that: it compiles with literal binds. + compiled = Video.length_seconds.expression.compile(dialect=postgresql.dialect()) + assert compiled.params == {} + assert f"> {LENGTH_MAX_S}" in str(compiled) + + +def test_a_years_long_archive_has_no_length_in_the_feed_or_a_playlist(db, make_user, as_user): + user = make_user() + pl = _seed(db, user) + client = as_user(user) + + feed = client.get("/api/feed?show=all&include_live=true&limit=50") + assert feed.status_code == 200 + by_id = {v["id"]: v["duration_seconds"] for v in feed.json()["items"]} + assert by_id["archive"] is None + assert by_id["day"] == LENGTH_MAX_S + + detail = client.get(f"/api/playlists/{pl.id}") + assert detail.status_code == 200 + items = {v["id"]: v["duration_seconds"] for v in detail.json()["items"]} + assert items["archive"] is None + + one = client.get("/api/videos/archive") + assert one.status_code == 200 + assert one.json()["duration_seconds"] is None + + +def test_the_totals_leave_it_out_and_count_the_live_ones(db, make_user, as_user): + user = make_user() + pl = _seed(db, user) + client = as_user(user) + + listed = client.get("/api/playlists") + assert listed.status_code == 200 + (summary,) = [p for p in listed.json() if p["id"] == pl.id] + assert summary["total_duration_seconds"] == KNOWN_SUM + assert summary["live_without_length"] == 2 # the archive and the one on air + + renamed = client.patch(f"/api/playlists/{pl.id}", json={"name": "Later 2"}) + assert renamed.status_code == 200 + assert renamed.json()["total_duration_seconds"] == KNOWN_SUM + assert renamed.json()["live_without_length"] == 2 + + channels = client.get("/api/channels") + assert channels.status_code == 200 + (ch,) = channels.json() + assert ch["total_duration_seconds"] == KNOWN_SUM + assert ch["live_without_length"] == 2 + + +def test_the_duration_filter_and_sort_pass_it_over(db, make_user, as_user): + user = make_user() + _seed(db, user) + client = as_user(user) + base = "/api/feed?show=all&include_live=true&limit=50" + + longer = client.get(f"{base}&min_duration={HOUR}") + assert longer.status_code == 200 + assert {v["id"] for v in longer.json()["items"]} == {"long", "day"} + + longest = client.get(f"{base}&sort=duration_desc") + assert longest.status_code == 200 + order = [v["id"] for v in longest.json()["items"]] + assert order[:3] == ["day", "long", "short"] + assert set(order[3:]) == {"archive", "onair", "unfetched"} + + +# Who may read `duration_seconds` itself, by (file, function): what each one asks is not "how long". +RAW_READERS = { + # The column is defined and turned into a length here. + ("catalog/models.py", "length_seconds"), + ("catalog/models.py", "_length_seconds_expression"), + # Ingest writes it; the live refresh asks whether YouTube has answered for an archive yet; the + # Shorts classification needs only "longer than a Short", which a years-long value proves. + ("sync/videos.py", "apply_video_details"), + ("sync/videos.py", "refresh_live"), + ("sync/videos.py", "run_shorts_classification"), + ("catalog/search.py", "classify_shorts"), + ("catalog/queries.py", "work_queue"), + # Not a video's column: the player's own report, and the watch history's looked-up lengths, which + # count a watched video as more than a glance (a watched archive is that too). + ("routes/_common.py", "kept_position"), + ("routes/plex/playback.py", "item_progress"), + ("reader/queries.py", "_counted_channels"), + ("reader/queries.py", "watched_channels"), + ("reader/queries.py", "watched_channel_videos"), +} + + +def _raw_reads() -> set[tuple[str, str]]: + root = Path(__file__).resolve().parents[1] / "app" + found: set[tuple[str, str]] = set() + for path in root.rglob("*.py"): + if "alembic" in path.parts: + continue + rel = path.relative_to(root).as_posix() + tree = ast.parse(path.read_text(encoding="utf-8")) + for fn in ast.walk(tree): + if not isinstance(fn, ast.FunctionDef): + continue + for node in ast.walk(fn): + if isinstance(node, ast.Attribute) and node.attr == "duration_seconds": + found.add((rel, fn.name)) + return found + + +def test_only_the_listed_readers_take_the_raw_column(): + found = _raw_reads() + stray = sorted(found - RAW_READERS) + assert not stray, f"read `length_seconds` (a length) instead of the raw column: {stray}" + # A listed reader that stopped reading it leaves the list. + assert not RAW_READERS - found, sorted(RAW_READERS - found) diff --git a/frontend/src/features/video/liveWithoutLength.test.ts b/frontend/src/features/video/liveWithoutLength.test.ts new file mode 100644 index 00000000..44819ba4 --- /dev/null +++ b/frontend/src/features/video/liveWithoutLength.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from "vitest"; +import type { TFunction } from "i18next"; +import { isLiveWithoutLength, withLiveCount } from "./liveWithoutLength"; + +const t = ((key: string, o: { count: number }) => `${key}:${o.count}`) as unknown as TFunction; + +describe("a total and the live videos it cannot hold", () => { + it("counts a live video only when the server sent no length", () => { + expect(isLiveWithoutLength({ duration_seconds: null, live_status: "was_live" })).toBe(true); + expect(isLiveWithoutLength({ duration_seconds: null, live_status: "live" })).toBe(true); + expect(isLiveWithoutLength({ duration_seconds: 5400, live_status: "was_live" })).toBe(false); + // Not fetched yet, and not live: left out of the sum like before, not named as live. + expect(isLiveWithoutLength({ duration_seconds: null, live_status: "none" })).toBe(false); + }); + + it("names them beside the sum, or alone when nothing summed", () => { + expect(withLiveCount("12 hr 5 min", 2, t)).toBe("12 hr 5 min card.liveWithoutLength:2"); + expect(withLiveCount("", 1, t)).toBe("card.liveWithoutLength:1"); + expect(withLiveCount("12 hr 5 min", 0, t)).toBe("12 hr 5 min"); + }); +}); diff --git a/frontend/src/features/video/liveWithoutLength.ts b/frontend/src/features/video/liveWithoutLength.ts new file mode 100644 index 00000000..b086b616 --- /dev/null +++ b/frontend/src/features/video/liveWithoutLength.ts @@ -0,0 +1,18 @@ +// A total of video lengths and the live videos it cannot hold. The server decides what a length is +// (a YouTube "length" past a day is a 24/7 stream's archive reporting the years it has run, and the +// API sends none for it: `catalog.models.LENGTH_MAX_S`), so this side keeps no copy of that limit. It +// only names what a sum left out: a live video — on air, upcoming, a premiere or an archive — with no +// length, as the server's `Video.live_without_length` counts it for the totals it sums itself. +import type { TFunction } from "i18next"; +import type { Video } from "../../lib/api/feed"; + +export function isLiveWithoutLength(v: Pick): boolean { + return v.duration_seconds == null && v.live_status !== "none"; +} + +/** "12 hr 5 min +2 live": `total` is the formatted sum ("" when nothing summed), `live` the count + * it left out. Empty when both are. */ +export function withLiveCount(total: string, live: number, t: TFunction): string { + const suffix = live > 0 ? t("card.liveWithoutLength", { count: live }) : ""; + return [total, suffix].filter(Boolean).join(" "); +} diff --git a/frontend/src/i18n/locales/en/card.json b/frontend/src/i18n/locales/en/card.json index e566dbed..168a97db 100644 --- a/frontend/src/i18n/locales/en/card.json +++ b/frontend/src/i18n/locales/en/card.json @@ -16,6 +16,8 @@ "restart": "Restart", "play": "Play", "stream": "stream", + "liveWithoutLength_one": "+{{count}} live", + "liveWithoutLength_other": "+{{count}} live", "views": "views", "info": "Details", "noDescription": "No description", diff --git a/frontend/src/i18n/locales/hu/card.json b/frontend/src/i18n/locales/hu/card.json index 80d8a9b8..ceb2e26b 100644 --- a/frontend/src/i18n/locales/hu/card.json +++ b/frontend/src/i18n/locales/hu/card.json @@ -16,6 +16,8 @@ "restart": "Elölről", "play": "Lejátszás", "stream": "közvetítés", + "liveWithoutLength_one": "+{{count}} élő", + "liveWithoutLength_other": "+{{count}} élő", "views": "megtekintés", "info": "Részletek", "noDescription": "Nincs leírás", diff --git a/frontend/src/lib/api/channels.ts b/frontend/src/lib/api/channels.ts index 7ca3c755..be9626c1 100644 --- a/frontend/src/lib/api/channels.ts +++ b/frontend/src/lib/api/channels.ts @@ -77,6 +77,8 @@ export interface ManagedChannel { * subscribe made here. */ added_at: string; total_duration_seconds: number; + /** Live videos the total cannot hold (no length): the "+N live" beside it. */ + live_without_length: number; priority: number; hidden: boolean; deep_requested: boolean; diff --git a/frontend/src/lib/api/playlists.ts b/frontend/src/lib/api/playlists.ts index 4150d508..2dfb2cea 100644 --- a/frontend/src/lib/api/playlists.ts +++ b/frontend/src/lib/api/playlists.ts @@ -17,6 +17,8 @@ export interface Playlist { dirty: boolean; // linked local playlist has edits not yet pushed to YouTube item_count: number; total_duration_seconds: number; + /** Live videos the total cannot hold (no length): the "+N live" beside it. */ + live_without_length: number; cover_thumbnail: string | null; updated_at: string; // ISO; when the list last CHANGED (rename, add/remove/move/reorder) has_video?: boolean; // only set when listed with ?contains= diff --git a/frontend/src/modules/channels/channelColumns.dom.test.tsx b/frontend/src/modules/channels/channelColumns.dom.test.tsx index 80104292..eb2b6b05 100644 --- a/frontend/src/modules/channels/channelColumns.dom.test.tsx +++ b/frontend/src/modules/channels/channelColumns.dom.test.tsx @@ -28,6 +28,7 @@ const ch = (over: Partial): ManagedChannel => ({ last_video_at: null, added_at: "2026-06-10T08:00:00+00:00", total_duration_seconds: 0, + live_without_length: 0, priority: 0, hidden: false, deep_requested: false, diff --git a/frontend/src/modules/channels/channelColumns.tsx b/frontend/src/modules/channels/channelColumns.tsx index ab2b9abc..ee7f6939 100644 --- a/frontend/src/modules/channels/channelColumns.tsx +++ b/frontend/src/modules/channels/channelColumns.tsx @@ -11,6 +11,7 @@ import { ChannelTagsControl, type SuggestedTag } from "../../features/channel/Ch import { StoredCell } from "./channelCells"; import { isFullySynced } from "./channelSync"; import { RelativeTime } from "../../components/ui/RelativeTime"; +import { withLiveCount } from "../../features/video/liveWithoutLength"; import type { FilterChip } from "./TagFilterChips"; /** The shared "subscribers" DataTable column (right-aligned, sortable, em-dash for null) — identical @@ -236,6 +237,10 @@ export function buildChannelColumns(deps: ChannelColumnDeps): Column ( {formatTotalHours(c.total_duration_seconds, lang)} + {/* Its own line: the column is sized for the sum alone. */} + {c.live_without_length > 0 && ( + {withLiveCount("", c.live_without_length, t)} + )} ), }, diff --git a/frontend/src/modules/channels/useChannelActions.dom.test.tsx b/frontend/src/modules/channels/useChannelActions.dom.test.tsx index 66d0bd32..1525590d 100644 --- a/frontend/src/modules/channels/useChannelActions.dom.test.tsx +++ b/frontend/src/modules/channels/useChannelActions.dom.test.tsx @@ -61,6 +61,7 @@ const ch = (over: Partial = {}): ManagedChannel => ({ last_video_at: null, added_at: "2026-06-10T08:00:00+00:00", total_duration_seconds: 0, + live_without_length: 0, priority: 0, hidden: false, deep_requested: false, diff --git a/frontend/src/modules/playlists/Playlists.dom.test.tsx b/frontend/src/modules/playlists/Playlists.dom.test.tsx index 3e3b2dcf..67834d23 100644 --- a/frontend/src/modules/playlists/Playlists.dom.test.tsx +++ b/frontend/src/modules/playlists/Playlists.dom.test.tsx @@ -134,6 +134,7 @@ const vid = (id: string, position_seconds: number): Video => status: "new", position_seconds, duration_seconds: 3000, + live_status: "none", published_at: "2026-01-01T00:00:00Z", watch_url: `https://www.youtube.com/watch?v=${id}`, }) as unknown as Video; @@ -998,6 +999,17 @@ describe("Playlists · the header", () => { expect(await meta()).toBe("playlists.itemCount"); }); + it("names the live videos the sum cannot hold beside it", async () => { + // A 24/7 stream's archive comes with no length (the server's rule): it is named, not summed. + const archive = (id: string) => ({ ...untimed(id), live_status: "was_live" }) as Video; + mount({ 1: [vid("A", 0), archive("B")] }); + expect(await meta()).toBe("playlists.itemCount · 50 min card.liveWithoutLength"); + cleanup(); + + mount({ 1: [archive("C")] }); + expect(await meta()).toBe("playlists.itemCount · card.liveWithoutLength"); + }); + it("offers the view controls only once there is more than one row to arrange", async () => { mount({ 1: [vid("A", 0)] }); await screen.findByText("Video A"); diff --git a/frontend/src/modules/playlists/Playlists.tsx b/frontend/src/modules/playlists/Playlists.tsx index 9a35ec45..4603d1a3 100644 --- a/frontend/src/modules/playlists/Playlists.tsx +++ b/frontend/src/modules/playlists/Playlists.tsx @@ -68,6 +68,7 @@ import { playlistViewParamUpdates, } from "../../lib/playlistsUrlState"; import { formatRuntime } from "../../lib/format"; +import { isLiveWithoutLength, withLiveCount } from "../../features/video/liveWithoutLength"; import { applyView, sameOrder } from "../../lib/playlistView"; import { useOpenLinkedPlayer } from "../../features/player/useOpenLinkedPlayer"; const PlayerModal = lazy(() => import("../../features/player/PlayerModal")); @@ -565,11 +566,13 @@ function PlaylistDetail() { }, [items]); // Total playtime of the whole playlist (sums every item, matching the itemCount beside it — not - // the filtered `shown` set). Items with an unknown duration count as 0; hidden when nothing sums. + // the filtered `shown` set). Items with an unknown duration count as 0, and a live one is named + // beside the sum instead ("+2 live"); hidden when there is neither. const totalSeconds = useMemo( () => items.reduce((acc, v) => acc + (v.duration_seconds ?? 0), 0), [items], ); + const liveCount = useMemo(() => items.filter(isLiveWithoutLength).length, [items]); // The row's "…" opens the row menu under itself. Which row's menu is open is part of the // cell (aria-expanded), so the columns rebuild when it changes — a cheap array of closures. @@ -1619,7 +1622,8 @@ function PlaylistDetail() { > {t("playlists.itemCount", { count: items.length })} - {totalSeconds > 0 && ` · ${formatRuntime(totalSeconds, lang)}`} + {(totalSeconds > 0 || liveCount > 0) && + ` · ${withLiveCount(formatRuntime(totalSeconds, lang), liveCount, t)}`} {filterActive && ` · ${t("playlists.filteredCount", { count: shown.length })}`} diff --git a/frontend/src/modules/playlists/PlaylistsRail.drop.dom.test.tsx b/frontend/src/modules/playlists/PlaylistsRail.drop.dom.test.tsx index f308cf54..587396a1 100644 --- a/frontend/src/modules/playlists/PlaylistsRail.drop.dom.test.tsx +++ b/frontend/src/modules/playlists/PlaylistsRail.drop.dom.test.tsx @@ -75,6 +75,7 @@ const pl = (id: number, name: string) => yt_playlist_id: null, item_count: 1, total_duration_seconds: 0, + live_without_length: 0, cover_thumbnail: null, dirty: false, }) as unknown as Playlist; diff --git a/frontend/src/modules/playlists/PlaylistsRail.rename.dom.test.tsx b/frontend/src/modules/playlists/PlaylistsRail.rename.dom.test.tsx index ff2e5f38..ab67642f 100644 --- a/frontend/src/modules/playlists/PlaylistsRail.rename.dom.test.tsx +++ b/frontend/src/modules/playlists/PlaylistsRail.rename.dom.test.tsx @@ -80,6 +80,7 @@ const alpha = { source: "local", item_count: 3, total_duration_seconds: 0, + live_without_length: 0, cover_thumbnail: null, dirty: false, } as unknown as Playlist; diff --git a/frontend/src/modules/playlists/PlaylistsRail.tsx b/frontend/src/modules/playlists/PlaylistsRail.tsx index 5be62d58..4edac479 100644 --- a/frontend/src/modules/playlists/PlaylistsRail.tsx +++ b/frontend/src/modules/playlists/PlaylistsRail.tsx @@ -8,6 +8,7 @@ import { YouTubeIcon } from "../../lib/brandIcons"; import { api, type Playlist } from "../../lib/api"; import { compareText } from "../../lib/collate"; import { formatRuntime } from "../../lib/format"; +import { withLiveCount } from "../../features/video/liveWithoutLength"; import { msg, notify } from "../../lib/notifications"; import { notifyYouTubeActionError } from "../../lib/youtubeErrors"; import { LS } from "../../lib/storage"; @@ -378,8 +379,12 @@ export default function PlaylistsRail() {
{t("playlists.itemCount", { count: pl.item_count })} - {pl.total_duration_seconds > 0 && - ` · ${formatRuntime(pl.total_duration_seconds, lang)}`} + {(pl.total_duration_seconds > 0 || pl.live_without_length > 0) && + ` · ${withLiveCount( + formatRuntime(pl.total_duration_seconds, lang), + pl.live_without_length, + t, + )}`}
From 9310ff85bfd21ed6c3bb77d9c5b1b3553c61df2d Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 7 Nov 2026 02:41:30 +0100 Subject: [PATCH 05/23] feat(discover): Surprise me leans on what you watch, and stays a surprise The Home hero's surprise tier and the Plex Discover rail were uniform draws over the unseen. They are now weighted draws (`app.draw`, Efraimidis-Spirakis over the same md5 shuffle): a title's weight is 1 + its affinity. Plex: the genres of the latest watched titles (half-life 180 days, a show counts once) and the TMDB recommendations stored on them (`plex.affinity`). YouTube: the channels the user watches (half-life 90 days). One slot of three stays a blind draw, and with no history the order is exactly the old one. No visible change. The YouTube draw builds its pool once (a materialized CTE read twice): the scope is ~200k videos on a prod-sized catalog, and a second ordered pass doubled the cost. Measured on localdev: a reroll 210 -> 292 ms. --- CLAUDE.md | 2 +- backend/app/README.md | 4 +- backend/app/discover/queries.py | 128 +++++++++-- backend/app/draw.py | 72 +++++++ backend/app/plex/affinity.py | 120 +++++++++++ backend/app/plex/discover.py | 34 +-- backend/tests/test_surprise_affinity.py | 269 ++++++++++++++++++++++++ 7 files changed, 598 insertions(+), 31 deletions(-) create mode 100644 backend/app/draw.py create mode 100644 backend/app/plex/affinity.py create mode 100644 backend/tests/test_surprise_affinity.py diff --git a/CLAUDE.md b/CLAUDE.md index 4b6b62a3..0d16a1f3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,7 +65,7 @@ how it gets built. Concretely: `routes/*` (HTTP only: validation, auth-dependency, serialization) → domain packages `sync/`, `plex/`, `downloads/`, `reader/`, `discover/`, `catalog/`, `youtube/`, `issues/` (shared by routes + scheduler + worker) → flat core (`auth`, `apitokens`, `quota`, `jobruns`, `sysconfig`, `state`, `audit`, `notifications`, `userscope`, `ratelimit`, `bodylimit`, `progress`, `collation`, -`security`, `logs`, `logsink`, `diagnostics`, `debugsession`) → `models/db/config`. **Dependency direction is one-way: core never imports routes.** +`security`, `logs`, `logsink`, `diagnostics`, `debugsession`, `draw`) → `models/db/config`. **Dependency direction is one-way: core never imports routes.** The domain is four **units**, each owning its tables: Capture (`downloads/`), Reader (`reader/`, `discover/`, the playlist sync), Catalog (`catalog/`, the `sync/` ingests) and Plex (`plex/`). A unit's route does no database work: it reads through the unit's `queries`, writes through its `service` (which flushes) and commits. A unit's code reads another unit's tables diff --git a/backend/app/README.md b/backend/app/README.md index 66c583cc..d79b0c11 100644 --- a/backend/app/README.md +++ b/backend/app/README.md @@ -25,8 +25,8 @@ Three layers, by *who* runs the code — not by feature: background run's durable row, its words in `runstatus`), `notifications`, `userscope`, `ratelimit`, `progress`, `logs` (the one logging configuration), `logsink` (its `log_events` table), `diagnostics` (a user's consented log - level), `debugsession` (a recording that tags its user's lines, E16) and `collation` (A→Z in the - reader's alphabet, E30), … are single-file primitives many layers import. They depend on nothing above them (a core + level), `debugsession` (a recording that tags its user's lines, E16), `collation` (A→Z in the + reader's alphabet, E30) and `draw` (the weighted, seeded draw "Surprise me" deals from), … are single-file primitives many layers import. They depend on nothing above them (a core module must never import a `routes/…` module). **Extraction trigger: the SECOND consumer.** Keep new logic inline in its first caller. The moment a diff --git a/backend/app/discover/queries.py b/backend/app/discover/queries.py index c26e8342..26a4cadb 100644 --- a/backend/app/discover/queries.py +++ b/backend/app/discover/queries.py @@ -17,9 +17,25 @@ otherwise "Mást kérek" re-draws the Plex picks while the YouTube ones stand st here takes the feed's seeded-md5 idiom instead of `random()`. """ -from sqlalchemy import and_, func, or_, select +import json +from datetime import datetime, timezone + +from sqlalchemy import ( + Float, + Select, + and_, + cast, + func, + literal, + literal_column, + or_, + select, + union_all, +) +from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import Session, aliased +from app import draw from app.discover.budget import band from app.feedquery import ( feed_columns, @@ -33,7 +49,7 @@ from app.models import User from app.plex.models import PlexItem, PlexShow, PlexState from app.catalog.models import Channel, Video from app.reader.models import PlaylistItem, VideoState -from app.plex import rollup +from app.plex import affinity, rollup from app.plex.cards import display_title, leaf_card, movie_card, show_card from app.plex.query import apply_show_state_filter, content_libs from app.userscope import owned_by @@ -115,14 +131,93 @@ def yt_surprise(db: Session, user: User, budget: str, seed: int, limit: int) -> params = dict(_FEED_BASE, min_duration=lo, max_duration=hi, include_shorts=budget == "short") query, rank_expr = filtered_query(db, user, **params) keys = sort_keys("shuffle", seed, rank_expr) + # Over-fetch, then drop anything already started. The feed's `unwatched` scope keeps watched and # hidden out but deliberately keeps in-progress rows in (they belong in the feed). Here they must # go: the resume tier has already had its chance at them, and it may have passed precisely because # the REMAINING time did not fit the budget — offering the same item back as a surprise, whose # runtime does fit, would label a half-finished thing "Meglepetés" and contradict the question. - rows = db.execute(query.order_by(*order_by(keys)).limit(limit * 4)).all() - out = [serialize(r) for r in rows] - return [r for r in out if not r["position_seconds"]][:limit] + take = limit * 4 + + def unstarted(rows) -> list[dict]: + return [r for r in map(serialize, rows) if not r["position_seconds"]] + + channels = _channel_affinity(db, user) + if not channels: + return unstarted(db.execute(query.order_by(*order_by(keys)).limit(take)).all())[:limit] + + # Both orders from ONE pass over the scope. The pool is the whole feed scope (~200k videos on a + # prod-sized catalog, ~390 ms a pass), so a second ordered query for the weighted draw doubled + # the cost; a materialized pool of (id, shuffle key, weighted key) is read twice instead. + weight = literal_column("1.0") + CHANNEL_WEIGHT * func.coalesce( + cast(cast(literal(json.dumps(channels)), JSONB).op("->>")(Video.channel_id), Float), 0.0 + ) + shuffle = func.md5(func.concat(Video.id, str(seed))) + pool = ( + query.with_only_columns( + Video.id.label("vid"), + shuffle.label("plain"), + draw.weighted_key(draw.seeded_u(Video.id, seed), weight).label("leaning"), + ) + .cte("surprise_pool") + .prefix_with("MATERIALIZED") + ) + + def top(side: int, *order) -> Select: + part = ( + select(pool.c.vid, pool.c.plain, pool.c.leaning, literal(side).label("side")) + .order_by(*order, pool.c.vid) + .limit(take) + .subquery() + ) + return select(part) + + # One statement, so the pool is built once; each side's order is restored from its keys. + got = db.execute(union_all(top(0, pool.c.plain), top(1, pool.c.leaning, pool.c.plain))).all() + plain_ids = [ + r.vid for r in sorted((r for r in got if r.side == 0), key=lambda r: (r.plain, r.vid)) + ] + leaning_ids = [ + r.vid + for r in sorted((r for r in got if r.side == 1), key=lambda r: (r.leaning, r.plain, r.vid)) + ] + rows = { + r["id"]: r + for r in unstarted(db.execute(query.where(Video.id.in_({*plain_ids, *leaning_ids}))).all()) + } + blind = [rows[i] for i in plain_ids if i in rows] + leaning = [rows[i] for i in leaning_ids if i in rows] + return draw.blend(leaning, blind, seed, limit, key=lambda r: r["id"]) + + +# How much a channel the user watches most outweighs one they never watch (1 + this), and how far back +# the watched history counts: the latest WATCHED_RECENT watches, the recent ones weighing more. +CHANNEL_WEIGHT = 3.0 +WATCHED_RECENT = 300 +WATCHED_HALF_LIFE_DAYS = 90 + + +def _channel_affinity(db: Session, user: User) -> dict[str, float]: + """Each channel the user watched lately, scored 0..1 against the most watched: the sum of its + watches, each weighing half as much every WATCHED_HALF_LIFE_DAYS. Empty with no history.""" + rows = db.execute( + select(Video.channel_id, VideoState.watched_at) + .join(Video, Video.id == VideoState.video_id) + .where(owned_by(VideoState.user_id, user), VideoState.status == "watched") + .order_by(VideoState.watched_at.desc().nullslast()) + .limit(WATCHED_RECENT) + ).all() + now = datetime.now(timezone.utc) + scores: dict[str, float] = {} + for channel_id, watched_at in rows: + age = ( + (now - watched_at).total_seconds() / 86400 if watched_at else 2 * WATCHED_HALF_LIFE_DAYS + ) + scores[channel_id] = scores.get(channel_id, 0.0) + 0.5 ** ( + max(0.0, age) / WATCHED_HALF_LIFE_DAYS + ) + top = max(scores.values(), default=0.0) + return {k: v / top for k, v in scores.items()} if top else {} def yt_video_row(db: Session, user: User, video_id: str) -> dict | None: @@ -289,21 +384,24 @@ def plex_surprise( if not movie_libs: return [] lo, hi = band(budget) - q = ( - select(PlexItem) - .where( - PlexItem.kind == "movie", - PlexItem.library_id.in_(movie_libs), - _unseen_movie_conds(uid), - ) - .order_by(_seeded(PlexItem.rating_key, seed)) - .limit(limit) + q = select(PlexItem).where( + PlexItem.kind == "movie", + PlexItem.library_id.in_(movie_libs), + _unseen_movie_conds(uid), ) if lo is not None: q = q.where(PlexItem.duration_s >= lo) if hi is not None: q = q.where(PlexItem.duration_s <= hi) - items = db.execute(q).scalars().all() + shuffled = _seeded(PlexItem.rating_key, seed) + blind = db.execute(q.order_by(shuffled, PlexItem.id).limit(limit)).scalars().all() + leaning = affinity.for_user(db, uid) + if leaning: + key = draw.weighted_key(draw.seeded_u(PlexItem.rating_key, seed), leaning.weight(PlexItem)) + weighted = db.execute(q.order_by(key, shuffled, PlexItem.id).limit(limit)).scalars().all() + items = draw.blend(list(weighted), list(blind), seed, limit, key=lambda it: it.id) + else: + items = list(blind) states = rollup.item_state_map(db, uid, [it.id for it in items]) return [movie_card(it, states.get(it.id), lang=lang) for it in items] diff --git a/backend/app/draw.py b/backend/app/draw.py new file mode 100644 index 00000000..25ce8747 --- /dev/null +++ b/backend/app/draw.py @@ -0,0 +1,72 @@ +"""A weighted draw that stays a surprise: what "Surprise me" deals from. + +A title's chance to come first is its weight over the pool's (Efraimidis–Spirakis: order by +-ln(1-u)/w, u uniform per row). The weight is 1 + the title's affinity to what the user watches +(`plex.affinity`, `discover.queries`), so a title nobody leans towards still comes up — only less +often. Two things keep it a surprise rather than a ranking: + +- **u is the seed's md5 of the row's key**, the feed's shuffle idiom. With every weight 1 the order + is exactly the md5 order the draw had before it was weighted, so a user with no history sees the + old draw, and one seed still moves both sources of the Home hero together. A draw that is not + seeded (the Plex Discover rail) takes u from random(). +- **One slot of three is blind** (`blend`): dealt from the unweighted order, so the same few + favourites cannot fill the rail. +""" + +from collections.abc import Callable, Hashable +from typing import Any + +from sqlalchemy import BigInteger, Float, cast, func, literal +from sqlalchemy.dialects.postgresql import BIT +from sqlalchemy.orm import InstrumentedAttribute +from sqlalchemy.sql.elements import ColumnElement + +# 2**32: the first 8 hex digits of an md5 as a fraction of one. +_SPAN = 4294967296.0 + + +def seeded_u( + key: ColumnElement[Any] | InstrumentedAttribute[Any], seed: int +) -> ColumnElement[float]: + """A uniform u in [0, 1) per row, from md5(key || seed) — the order `md5(...)` ascending gives.""" + digest = func.md5(func.concat(key, str(seed))) + head = cast(cast(literal("x") + func.substr(digest, 1, 8), BIT(32)), BigInteger) + return cast(head, Float) / _SPAN + + +def weighted_key(u: ColumnElement[float], weight: ColumnElement[float]) -> ColumnElement[float]: + """The row's place in a weighted draw, ascending: -ln(1-u)/w. Monotonic in u when every w is 1, + so the unweighted order is unchanged.""" + return -func.ln(1.0 - u) / weight + + +def random_u() -> ColumnElement[float]: + return func.random() + + +def blend( + weighted: list, blind: list, offset: int, n: int, key: Callable[[Any], Hashable] = lambda x: x +) -> list: + """`n` picks: slot i comes from `blind` when (i + offset) % 3 == 0, else from `weighted`, each + skipping what an earlier slot took (`key` tells two picks apart). A side that runs out leaves the + rest to the other.""" + out: list = [] + taken: set = set() + sources = (iter(weighted), iter(blind)) + + def nxt(source): + for item in source: + if key(item) not in taken: + return item + return None + + for i in range(n): + first, second = (sources[1], sources[0]) if (i + offset) % 3 == 0 else sources + item = nxt(first) + if item is None: + item = nxt(second) + if item is None: + break + taken.add(key(item)) + out.append(item) + return out diff --git a/backend/app/plex/affinity.py b/backend/app/plex/affinity.py new file mode 100644 index 00000000..4d25fbd3 --- /dev/null +++ b/backend/app/plex/affinity.py @@ -0,0 +1,120 @@ +"""What a user's Plex history leans towards, as a weight for a surprise draw (`app.draw`). + +Two signals, both already in the database (no TMDB call): +- **genres** of the titles the user watched, the recent ones weighing more (a half-life of + HALF_LIFE_DAYS), each scored 0..1 against the strongest; +- the **TMDB recommendations** stored on those titles (`recommendations`), each scored by the most + recent title that recommends it. + +A title counts once however many of its episodes were watched — a binge of one show would otherwise +outvote every film — and only the RECENT_TITLES latest titles count. A candidate's weight is +1 + GENRE_WEIGHT × (the mean score of its genres) + REC_WEIGHT × (its recommendation score), so a +title with no affinity keeps weight 1 and still comes up. +""" + +import json +from dataclasses import dataclass, field +from datetime import datetime, timezone + +from sqlalchemy import Float, cast, func, literal, literal_column, select +from sqlalchemy.dialects.postgresql import JSONB +from sqlalchemy.orm import Session +from sqlalchemy.sql.elements import ColumnElement + +from app.plex.models import PlexItem, PlexShow, PlexState +from app.userscope import owned_by + +HALF_LIFE_DAYS = 180 +RECENT_TITLES = 200 +GENRE_WEIGHT = 2.0 +REC_WEIGHT = 3.0 +# A watch with no date (an old import) counts as one this many half-lives ago. +_UNDATED_HALF_LIVES = 2 + + +@dataclass(frozen=True) +class Affinity: + genres: dict[str, float] = field(default_factory=dict) + movie_recs: dict[str, float] = field(default_factory=dict) + show_recs: dict[str, float] = field(default_factory=dict) + + def __bool__(self) -> bool: + return bool(self.genres or self.movie_recs or self.show_recs) + + def weight(self, model: type[PlexItem] | type[PlexShow]) -> ColumnElement[float]: + """The draw weight of a `model` row (a movie or a show).""" + scores = cast(literal(json.dumps(self.genres)), JSONB) + recs = self.show_recs if model is PlexShow else self.movie_recs + genre = func.jsonb_array_elements_text(model.genres).table_valued("value").alias("g") + genre_aff = ( + select( + func.coalesce( + func.avg(func.coalesce(cast(scores.op("->>")(genre.c.value), Float), 0.0)), + 0.0, + ) + ) + .select_from(genre) + .scalar_subquery() + ) + rec_aff = func.coalesce( + cast(cast(literal(json.dumps(recs)), JSONB).op("->>")(model.guid), Float), 0.0 + ) + return literal_column("1.0") + GENRE_WEIGHT * genre_aff + REC_WEIGHT * rec_aff + + +def for_user(db: Session, uid: int) -> Affinity: + """The user's affinity from their latest watched titles; empty (`not aff`) with no history.""" + # One key per title: a show for an episode, the item for a film. concat() of a NULL show_id is + # just "s", which nullif turns back into NULL so the film's key takes over. + title = func.coalesce( + func.nullif(func.concat("s", PlexItem.show_id), "s"), func.concat("m", PlexItem.id) + ).label("title") + latest = ( + select(title, PlexState.watched_at, PlexItem.id.label("item_id"), PlexItem.show_id) + .join(PlexItem, PlexItem.id == PlexState.item_id) + .where(owned_by(PlexState.user_id, uid), PlexState.status == "watched") + .distinct(title) + .order_by(title, PlexState.watched_at.desc().nullslast()) + .subquery() + ) + rows = db.execute( + select( + latest.c.watched_at, + PlexItem.genres, + PlexItem.recommendations, + PlexShow.genres, + PlexShow.recommendations, + ) + .select_from(latest) + .join(PlexItem, PlexItem.id == latest.c.item_id) + .outerjoin(PlexShow, PlexShow.id == latest.c.show_id) + .order_by(latest.c.watched_at.desc().nullslast()) + .limit(RECENT_TITLES) + ).all() + if not rows: + return Affinity() + + now = datetime.now(timezone.utc) + genres: dict[str, float] = {} + movie_recs: dict[str, float] = {} + show_recs: dict[str, float] = {} + for watched_at, item_genres, item_recs, show_genres, show_recs_json in rows: + if watched_at is None: + r = 0.5**_UNDATED_HALF_LIVES + else: + r = 0.5 ** (max(0.0, (now - watched_at).total_seconds() / 86400) / HALF_LIFE_DAYS) + for g in (show_genres if show_genres is not None else item_genres) or []: + genres[g] = genres.get(g, 0.0) + r + for rec in (show_recs_json if show_recs_json is not None else item_recs) or []: + if not rec.get("tmdb_id"): + continue + bucket = show_recs if rec.get("media_type") == "tv" else movie_recs + guid = f"tmdb:{rec['tmdb_id']}" + bucket[guid] = max(bucket.get(guid, 0.0), r) + return Affinity(_scaled(genres), _scaled(movie_recs), _scaled(show_recs)) + + +def _scaled(scores: dict[str, float]) -> dict[str, float]: + """Each score over the strongest, so the strongest is 1.""" + top = max(scores.values(), default=0.0) + return {k: v / top for k, v in scores.items()} if top else {} diff --git a/backend/app/plex/discover.py b/backend/app/plex/discover.py index ac9ed64a..11461e4e 100644 --- a/backend/app/plex/discover.py +++ b/backend/app/plex/discover.py @@ -22,7 +22,8 @@ from sqlalchemy.orm import Session from app.collation import sorted_words from app.discover.queries import RAIL_LIMIT from app.models import User -from app.plex import rollup +from app import draw +from app.plex import affinity, rollup from app.plex.cards import SHOW_NEW, display_title, leaf_card, movie_card, show_card from app.plex.models import PlexItem, PlexShow, PlexState from app.plex.query import ( @@ -399,22 +400,25 @@ def discover_page(db: Session, user: User, scope: str, lang: str, f: LibraryFilt } -def _draw(db: Session, stmt, key_col, per: int, exclude: list[str]) -> list: +def _draw(db: Session, stmt, key_col, per: int, exclude: list[str], weight=None) -> list: """`per` random rows of `stmt`, the previous draw's (`exclude`) left out while the pool allows: a reshuffle over a small filtered pool kept dealing the same few titles. What the fresh rows leave - short is topped up from the excluded ones, so a pool smaller than a draw still fills the rail.""" - fresh = ( - db.execute(stmt.where(key_col.not_in(exclude)).order_by(func.random()).limit(per)) - .scalars() - .all() - if exclude - else [] - ) + short is topped up from the excluded ones, so a pool smaller than a draw still fills the rail. + With a `weight` (`plex.affinity`) the draw leans on it, every third slot left blind (`app.draw`).""" + + def pick(q, n: int) -> list: + blind = db.execute(q.order_by(func.random()).limit(n)).scalars().all() + if weight is None: + return list(blind) + key = draw.weighted_key(draw.random_u(), weight) + leaning = db.execute(q.order_by(key).limit(n)).scalars().all() + return draw.blend(list(leaning), list(blind), 1, n, key=lambda r: r.rating_key) + + fresh = pick(stmt.where(key_col.not_in(exclude)), per) if exclude else [] if len(fresh) == per: - return list(fresh) + return fresh taken = [key_col.not_in([r.rating_key for r in fresh])] if fresh else [] - rest = db.execute(stmt.where(*taken).order_by(func.random()).limit(per - len(fresh))).scalars() - return [*fresh, *rest.all()] + return [*fresh, *pick(stmt.where(*taken), per - len(fresh))] def _kept( @@ -474,6 +478,8 @@ def surprise( if keep: return {"scope": scope, "items": _kept(db, user.id, movie_ids, show_ids, keep, lang)} per = min(limit, SURPRISE_PER_KIND) + # What the user's history leans towards; with none, every title weighs the same (a plain draw). + leaning = affinity.for_user(db, user.id) movies = [] if movie_ids: # Exclude movies this user has watched. @@ -501,6 +507,7 @@ def surprise( PlexItem.rating_key, per, exclude, + leaning.weight(PlexItem) if leaning else None, ) shows = [] if show_ids: @@ -523,6 +530,7 @@ def surprise( PlexShow.rating_key, per, exclude, + leaning.weight(PlexShow) if leaning else None, ) mcards = _movie_cards(db, user.id, movies, lang) scards = _show_cards(db, user.id, shows, lang) diff --git a/backend/tests/test_surprise_affinity.py b/backend/tests/test_surprise_affinity.py new file mode 100644 index 00000000..0921c02c --- /dev/null +++ b/backend/tests/test_surprise_affinity.py @@ -0,0 +1,269 @@ +"""Surprise me leans on what the user watches, and stays a surprise. + +The Home hero's surprise tier and the Plex Discover rail were uniform draws over the unseen. They are +now weighted draws (`app.draw`): a title's weight is 1 + its affinity — for Plex the genres of the +films and shows the user watched (recent ones weigh more) and the TMDB recommendations stored on them; +for YouTube the channels the user watches. One slot of three stays a blind draw, and with no watch +history the order is exactly the old one. DB lane. +""" + +import hashlib +from datetime import datetime, timedelta, timezone + +from sqlalchemy import text + +from app import draw +from app.catalog.models import Channel, Subscription, Video +from app.discover import queries +from app.plex import affinity, discover as plex_discover +from app.plex.models import PlexItem, PlexLibrary, PlexShow, PlexState +from app.plex.query import LibraryFilters +from app.reader.models import VideoState + +NOW = datetime.now(timezone.utc) +SEEDS = range(90) + + +# ── the draw itself ────────────────────────────────────────────────────────────────────────────── + + +def test_every_third_slot_is_blind_and_nothing_repeats(): + weighted = ["a", "b", "c", "d", "e", "f"] + blind = ["f", "e", "d", "c", "b", "a"] + # offset 0: slots 0 and 3 are blind + assert draw.blend(weighted, blind, 0, 6) == ["f", "a", "b", "e", "c", "d"] + # offset 1: slot 2 and 5 + assert draw.blend(weighted, blind, 1, 6) == ["a", "b", "f", "c", "d", "e"] + assert draw.blend(weighted, blind, 2, 3) == ["a", "f", "b"] + + +def test_a_short_side_is_topped_up_from_the_other(): + assert draw.blend(["a"], ["a", "b", "c"], 0, 3) == ["a", "b", "c"] + assert draw.blend(["a", "b", "c"], [], 0, 3) == ["a", "b", "c"] + + +# ── YouTube: the channels the user watches ─────────────────────────────────────────────────────── + + +def _md5_order(ids, seed): + return sorted(ids, key=lambda i: hashlib.md5(f"{i}{seed}".encode()).hexdigest()) + + +def _seed_yt(db, make_user, *, watched_in_fav: int): + user = make_user() + fav, other = Channel(id="UCfav", title="Fav"), Channel(id="UCother", title="Other") + db.add_all([fav, other]) + db.flush() + db.add_all( + [ + Subscription(user_id=user.id, channel_id="UCfav"), + Subscription(user_id=user.id, channel_id="UCother"), + ] + ) + + def video(vid, ch, days): + return Video( + id=vid, + channel_id=ch, + title=vid, + duration_seconds=20 * 60, + published_at=NOW - timedelta(days=days), + ) + + # One unseen video per channel, plus the user's watched history on the favourite. + db.add_all([video("fav_new", "UCfav", 1), video("other_new", "UCother", 1)]) + for i in range(watched_in_fav): + db.add(video(f"fav_seen{i}", "UCfav", 10 + i)) + db.flush() + for i in range(watched_in_fav): + db.add( + VideoState( + user_id=user.id, + video_id=f"fav_seen{i}", + status="watched", + watched_at=NOW - timedelta(days=i), + ) + ) + db.commit() + return user + + +def _first(rows): + return rows[0]["id"] if rows else None + + +def test_a_watched_channel_wins_most_draws_but_not_all(db, make_user): + user = _seed_yt(db, make_user, watched_in_fav=12) + leads = [_first(queries.yt_surprise(db, user, "any", s, 2)) for s in SEEDS] + fav = leads.count("fav_new") / len(leads) + # weight 1 + 3 against 1: 0.8 in a weighted slot, 0.5 in the blind one (a third of the seeds) + assert 0.6 < fav < 0.9, fav + + +def test_the_blind_slot_deals_the_plain_shuffle_however_strong_the_lean(db, make_user): + user = _seed_yt(db, make_user, watched_in_fav=12) + for s in range(0, 30, 3): # (0 + seed) % 3 == 0: the first slot is the blind one + got = _first(queries.yt_surprise(db, user, "any", s, 2)) + assert got == _md5_order(["fav_new", "other_new"], s)[0] + + +def test_with_no_history_the_youtube_order_is_the_old_shuffle(db, make_user): + user = _seed_yt(db, make_user, watched_in_fav=0) + for s in range(12): + got = [r["id"] for r in queries.yt_surprise(db, user, "any", s, 2)] + assert got == _md5_order(["fav_new", "other_new"], s) + + +def test_a_watched_video_is_never_drawn(db, make_user): + user = _seed_yt(db, make_user, watched_in_fav=12) + drawn = {r["id"] for s in SEEDS for r in queries.yt_surprise(db, user, "any", s, 5)} + assert drawn == {"fav_new", "other_new"} + + +# ── Plex: genres and recommendations of what the user watched ──────────────────────────────────── + + +def _seed_plex(db, make_user, *, history: bool): + user = make_user() + lib = PlexLibrary(plex_key="77", title="Movies", kind="movie", enabled=True) + db.add(lib) + db.flush() + tmdb_ids = iter(range(5000, 6000)) + + def movie(key, genres, recs=None): + m = PlexItem( + rating_key=key, + library_id=lib.id, + kind="movie", + title=key, + duration_s=100 * 60, + genres=genres, + guid=f"tmdb:{next(tmdb_ids)}", + recommendations=recs, + ) + db.add(m) + return m + + horror = movie("horror_new", ["Horror"]) + comedy = movie("comedy_new", ["Comedy"]) + tipped = movie("tipped_new", ["Western"]) + db.flush() + tip = [{"tmdb_id": int(tipped.guid.split(":")[1]), "media_type": "movie", "title": "t"}] + seen = [movie(f"horror_seen{i}", ["Horror"], tip if i == 0 else None) for i in range(3)] + db.flush() + if history: + for i, m in enumerate(seen): + db.add( + PlexState( + user_id=user.id, + item_id=m.id, + status="watched", + watched_at=NOW - timedelta(days=i), + ) + ) + db.commit() + from app import sysconfig + + sysconfig.set_value(db, "plex_enabled", True) + db.commit() + return user, horror, comedy, tipped + + +def test_the_genre_and_the_recommendation_of_what_was_watched_win_most_draws(db, make_user): + user, *_ = _seed_plex(db, make_user, history=True) + leads = [_first(queries.plex_surprise(db, user.id, "any", s, 3, lang="en")) for s in SEEDS] + # Weights: Horror 1 + 2 (its genre), the recommended Western 1 + 3, the Comedy nobody chose 1. In + # a weighted slot the Comedy leads 1 time in 8, in the blind third 1 in 3: about 0.19 against + # 0.81 for the other two together (a uniform draw: 1 in 3 each). It still leads now and then — + # a surprise, not a ranking. + comedy = leads.count("comedy_new") + assert comedy > 0 + assert leads.count("horror_new") > 1.5 * comedy and leads.count("tipped_new") > 1.5 * comedy + assert leads.count("horror_new") + leads.count("tipped_new") > 3 * comedy + + +def test_with_no_history_the_plex_order_is_the_old_shuffle(db, make_user): + user, *_ = _seed_plex(db, make_user, history=False) + keys = ["horror_new", "comedy_new", "tipped_new"] + [f"horror_seen{i}" for i in range(3)] + for s in range(12): + got = [c["id"] for c in queries.plex_surprise(db, user.id, "any", s, 6, lang="en")] + assert got == _md5_order(keys, s) + + +def test_a_watched_film_is_never_drawn(db, make_user): + user, *_ = _seed_plex(db, make_user, history=True) + drawn = { + c["id"] for s in SEEDS for c in queries.plex_surprise(db, user.id, "any", s, 6, lang="en") + } + assert drawn == {"horror_new", "comedy_new", "tipped_new"} + + +def test_the_plex_blind_slot_deals_the_plain_shuffle(db, make_user): + user, *_ = _seed_plex(db, make_user, history=True) + unseen = ["horror_new", "comedy_new", "tipped_new"] + for s in range(0, 30, 3): + got = _first(queries.plex_surprise(db, user.id, "any", s, 3, lang="en")) + assert got == _md5_order(unseen, s)[0] + + +def test_a_binged_show_counts_once_and_recent_watches_weigh_more(db, make_user): + user = make_user() + lib = PlexLibrary(plex_key="78", title="Shows", kind="show", enabled=True) + mlib = PlexLibrary(plex_key="79", title="Movies", kind="movie", enabled=True) + db.add_all([lib, mlib]) + db.flush() + show = PlexShow(rating_key="sitcom", library_id=lib.id, title="Sitcom", genres=["Comedy"]) + db.add(show) + db.flush() + eps = [ + PlexItem( + rating_key=f"ep{i}", + library_id=lib.id, + kind="episode", + title=f"e{i}", + show_id=show.id, + season_number=1, + episode_number=i, + ) + for i in range(10) + ] + films = [ + PlexItem( + rating_key=f"h{i}", library_id=mlib.id, kind="movie", title=f"h{i}", genres=["Horror"] + ) + for i in range(2) + ] + old = PlexItem( + rating_key="w0", library_id=mlib.id, kind="movie", title="w0", genres=["Western"] + ) + db.add_all([*eps, *films, old]) + db.flush() + for it in [*eps, *films]: + db.add(PlexState(user_id=user.id, item_id=it.id, status="watched", watched_at=NOW)) + db.add( + PlexState( + user_id=user.id, item_id=old.id, status="watched", watched_at=NOW - timedelta(days=720) + ) + ) + db.commit() + + genres = affinity.for_user(db, user.id).genres + # Ten episodes of one show are one title: two horror films outweigh it. + assert genres["Horror"] == 1.0 + assert genres["Comedy"] == 0.5 + # Four half-lives ago weighs a sixteenth of today. + assert abs(genres["Western"] - 0.5 / 16) < 0.01 + + +def test_the_discover_rail_leans_the_same_way(db, make_user): + # The rail draws with random(), not a seed; setseed makes this session's draws repeatable. + user, *_ = _seed_plex(db, make_user, history=True) + db.execute(text("SELECT setseed(0.42)")) + leads = [ + plex_discover.surprise(db, user, "movie", "en", 3, [], [], LibraryFilters())["items"][0][ + "id" + ] + for _ in SEEDS + ] + assert leads.count("comedy_new") > 0 + assert leads.count("horror_new") + leads.count("tipped_new") > 3 * leads.count("comedy_new") From 25aed6635ff056d97464309fb3aff12293ec89af Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 7 Nov 2026 04:00:00 +0100 Subject: [PATCH 06/23] perf(feed): a shuffle walks a stored random key instead of sorting the whole scope Every video carries a random number drawn once (videos.shuffle_key, migration 0118, indexed with id). A seed picks a point on that ring: the feed's Shuffle sort pages along it and wraps, and the Home surprise reads a 500-video window of it and shuffles that window by the seed, so no video is favoured for sitting after a wide gap. The md5(id || seed) order read and sorted the whole feed scope on every call: ~350 ms on prod per surprise, 381-767 ms per Shuffle page on localdev, now 14 ms and 4 ms. --- backend/.basedpyright/baseline.json | 112 ------------- .../versions/0118_videos_shuffle_key.py | 40 +++++ backend/app/catalog/models.py | 4 + backend/app/discover/queries.py | 112 ++++++------- backend/app/draw.py | 26 +++ backend/app/feedquery.py | 68 +++++++- backend/tests/test_shuffle_walk.py | 151 ++++++++++++++++++ 7 files changed, 328 insertions(+), 185 deletions(-) create mode 100644 backend/alembic/versions/0118_videos_shuffle_key.py create mode 100644 backend/tests/test_shuffle_walk.py diff --git a/backend/.basedpyright/baseline.json b/backend/.basedpyright/baseline.json index af71727c..75d496cf 100644 --- a/backend/.basedpyright/baseline.json +++ b/backend/.basedpyright/baseline.json @@ -185,118 +185,6 @@ "lineCount": 1 } }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 50, - "endColumn": 56, - "lineCount": 1 - } - }, { "code": "reportArgumentType", "range": { diff --git a/backend/alembic/versions/0118_videos_shuffle_key.py b/backend/alembic/versions/0118_videos_shuffle_key.py new file mode 100644 index 00000000..683b5a76 --- /dev/null +++ b/backend/alembic/versions/0118_videos_shuffle_key.py @@ -0,0 +1,40 @@ +"""videos: a random key per video, so a seeded shuffle walks an index instead of sorting the scope + +Revision ID: 0118_videos_shuffle_key +Revises: 0117_videos_length_sort_indexes +Create Date: 2026-11-07 + +The feed's Shuffle sort and the Home surprise ordered the whole feed scope by md5(id || seed) on every +call: ~350 ms on prod for a scope of ~213k videos, and a Shuffle page deeper in the list cost more +(767 ms on page 7 on localdev, against 4 ms for Newest). Each video now carries a random number drawn +once (`Video.shuffle_key`); a seed picks a point on that ring and the draw reads the index from there +(`feedquery.shuffle_walk`). + +One ADD COLUMN with a volatile default rewrites the table and fills every row with its own value. On +localdev (278k videos) that took 28 s, against 84 s for an UPDATE of a nullable column, so the rewrite +it is: the table is locked for that time, and a deploy has the app down while it migrates anyway. +The index is (shuffle_key, id): the walk's keyset cursor compares the pair. +""" + +from alembic import op + +revision: str = "0118_videos_shuffle_key" +down_revision: str | None = "0117_videos_length_sort_indexes" +branch_labels: str | None = None +depends_on: str | None = None + + +def upgrade() -> None: + op.execute( + "ALTER TABLE videos ADD COLUMN IF NOT EXISTS shuffle_key double precision " + "NOT NULL DEFAULT random()" + ) + with op.get_context().autocommit_block(): + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_shuffle_key") + op.execute("CREATE INDEX CONCURRENTLY ix_videos_shuffle_key ON videos (shuffle_key, id)") + + +def downgrade() -> None: + with op.get_context().autocommit_block(): + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_shuffle_key") + op.execute("ALTER TABLE videos DROP COLUMN IF EXISTS shuffle_key") diff --git a/backend/app/catalog/models.py b/backend/app/catalog/models.py index a55a9e84..daf44bcb 100644 --- a/backend/app/catalog/models.py +++ b/backend/app/catalog/models.py @@ -10,6 +10,7 @@ from sqlalchemy import ( Boolean, Computed, DateTime, + Float, ForeignKey, Integer, JSON, @@ -217,6 +218,9 @@ class Video(Base, TimestampMixin): last_checked_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) unavailable_since: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) unavailable_reason: Mapped[str | None] = mapped_column(String(24)) + # A random number in [0, 1), drawn once when the row is written: the ring a seeded shuffle walks + # (`feedquery.shuffle_walk`, index ix_videos_shuffle_key on (shuffle_key, id), migration 0118). + shuffle_key: Mapped[float] = mapped_column(Float, nullable=False, server_default=func.random()) @hybrid_property def length_seconds(self) -> int | None: diff --git a/backend/app/discover/queries.py b/backend/app/discover/queries.py index 26a4cadb..f83e6498 100644 --- a/backend/app/discover/queries.py +++ b/backend/app/discover/queries.py @@ -10,29 +10,22 @@ explore-visible and in-progress for free, and cannot drift from them. The Plex h narrows EVERY rail by the Plex sidebar's `LibraryFilters`, which is right for a surface that HAS a sidebar and wrong for a landing page that does not. -**One seed drives both halves.** The feed orders a shuffle by `md5(id || seed)` so a keyset cursor -stays valid across pages; the Plex surprise rail uses `ORDER BY random()`, which is fine for a single -un-paged draw but cannot be reproduced. A cross-source reroll needs both halves to move together — -otherwise "Mást kérek" re-draws the Plex picks while the YouTube ones stand still — so the Plex side -here takes the feed's seeded-md5 idiom instead of `random()`. +**One seed drives both halves.** The YouTube half starts on the ring of stored random keys at the +seed's point and shuffles that window by `md5(id || seed)`; the Plex surprise rail uses +`ORDER BY random()`, which is fine for a single un-paged draw but cannot be reproduced. A +cross-source reroll needs both halves to move together — otherwise "Mást kérek" re-draws the Plex +picks while the YouTube ones stand still — so the Plex side here orders by the same seeded md5 +instead of `random()`. """ -import json from datetime import datetime, timezone from sqlalchemy import ( - Float, - Select, and_, - cast, func, - literal, - literal_column, or_, select, - union_all, ) -from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import Session, aliased from app import draw @@ -43,6 +36,7 @@ from app.feedquery import ( not_blocked_cond, order_by, serialize, + shuffle_walk, sort_keys, ) from app.models import User @@ -128,66 +122,50 @@ def yt_surprise(db: Session, user: User, budget: str, seed: int, limit: int) -> Shorts are opened up only for the shortest band: "I have ten minutes" is the one answer a Short is actually a good response to, and the feed's own default keeps them out otherwise.""" lo, hi = band(budget) - params = dict(_FEED_BASE, min_duration=lo, max_duration=hi, include_shorts=budget == "short") - query, rank_expr = filtered_query(db, user, **params) - keys = sort_keys("shuffle", seed, rank_expr) - - # Over-fetch, then drop anything already started. The feed's `unwatched` scope keeps watched and - # hidden out but deliberately keeps in-progress rows in (they belong in the feed). Here they must - # go: the resume tier has already had its chance at them, and it may have passed precisely because - # the REMAINING time did not fit the budget — offering the same item back as a surprise, whose - # runtime does fit, would label a half-finished thing "Meglepetés" and contradict the question. - take = limit * 4 - - def unstarted(rows) -> list[dict]: - return [r for r in map(serialize, rows) if not r["position_seconds"]] + params: dict = { + **_FEED_BASE, + "min_duration": lo, + "max_duration": hi, + "include_shorts": budget == "short", + } + query, _rank = filtered_query(db, user, **params) + # A window of the ring, not the whole scope: the seed's SURPRISE_WINDOW videos read along the + # stored random key (`shuffle_walk`, a millisecond), then shuffled by the seed. Dealing straight + # from the ring would favour for good every video that sits after a wide gap of it (the first + # video past a random point is the one whose gap the point fell into); shuffling the window makes + # each video's chance the window's share, even to within a few percent. A scope no larger than + # the window is the whole scope, in exactly the md5 order the draw always had. + # + # Anything already started is dropped. The feed's `unwatched` scope keeps watched and hidden out + # but deliberately keeps in-progress rows in (they belong in the feed). Here they must go: the + # resume tier has already had its chance at them, and it may have passed precisely because the + # REMAINING time did not fit the budget — offering the same item back as a surprise, whose runtime + # does fit, would label a half-finished thing "Meglepetés" and contradict the question. + window = [ + r + for r in map(serialize, shuffle_walk(db, query, seed, SURPRISE_WINDOW)) + if not r["position_seconds"] + ] + blind = draw.shuffled(window, seed, key=_video_id) channels = _channel_affinity(db, user) if not channels: - return unstarted(db.execute(query.order_by(*order_by(keys)).limit(take)).all())[:limit] + return blind[:limit] - # Both orders from ONE pass over the scope. The pool is the whole feed scope (~200k videos on a - # prod-sized catalog, ~390 ms a pass), so a second ordered query for the weighted draw doubled - # the cost; a materialized pool of (id, shuffle key, weighted key) is read twice instead. - weight = literal_column("1.0") + CHANNEL_WEIGHT * func.coalesce( - cast(cast(literal(json.dumps(channels)), JSONB).op("->>")(Video.channel_id), Float), 0.0 - ) - shuffle = func.md5(func.concat(Video.id, str(seed))) - pool = ( - query.with_only_columns( - Video.id.label("vid"), - shuffle.label("plain"), - draw.weighted_key(draw.seeded_u(Video.id, seed), weight).label("leaning"), - ) - .cte("surprise_pool") - .prefix_with("MATERIALIZED") - ) + def weight(row: dict) -> float: + return 1.0 + CHANNEL_WEIGHT * channels.get(row["channel_id"], 0.0) - def top(side: int, *order) -> Select: - part = ( - select(pool.c.vid, pool.c.plain, pool.c.leaning, literal(side).label("side")) - .order_by(*order, pool.c.vid) - .limit(take) - .subquery() - ) - return select(part) + leaning = draw.leaning(window, seed, key=_video_id, weight=weight) + return draw.blend(leaning, blind, seed, limit, key=_video_id) - # One statement, so the pool is built once; each side's order is restored from its keys. - got = db.execute(union_all(top(0, pool.c.plain), top(1, pool.c.leaning, pool.c.plain))).all() - plain_ids = [ - r.vid for r in sorted((r for r in got if r.side == 0), key=lambda r: (r.plain, r.vid)) - ] - leaning_ids = [ - r.vid - for r in sorted((r for r in got if r.side == 1), key=lambda r: (r.leaning, r.plain, r.vid)) - ] - rows = { - r["id"]: r - for r in unstarted(db.execute(query.where(Video.id.in_({*plain_ids, *leaning_ids}))).all()) - } - blind = [rows[i] for i in plain_ids if i in rows] - leaning = [rows[i] for i in leaning_ids if i in rows] - return draw.blend(leaning, blind, seed, limit, key=lambda r: r["id"]) + +# How many videos of the ring a surprise draws from: wide enough that a channel the user watches has +# a few in it, small enough to read in a millisecond or two. +SURPRISE_WINDOW = 500 + + +def _video_id(row: dict) -> str: + return row["id"] # How much a channel the user watches most outweighs one they never watch (1 + this), and how far back diff --git a/backend/app/draw.py b/backend/app/draw.py index 25ce8747..7281501d 100644 --- a/backend/app/draw.py +++ b/backend/app/draw.py @@ -11,8 +11,13 @@ often. Two things keep it a surprise rather than a ranking: seeded (the Plex Discover rail) takes u from random(). - **One slot of three is blind** (`blend`): dealt from the unweighted order, so the same few favourites cannot fill the rail. + +The YouTube surprise draws over a window of the feed scope rather than all of it (`yt_surprise`): +the window is read in SQL, and `shuffled` / `leaning` order it here with the same u. """ +import hashlib +import math from collections.abc import Callable, Hashable from typing import Any @@ -44,6 +49,27 @@ def random_u() -> ColumnElement[float]: return func.random() +def u_of(key: str, seed: int) -> float: + """`seeded_u` in Python: the same u for the same key and seed.""" + return int(hashlib.md5(f"{key}{seed}".encode()).hexdigest()[:8], 16) / _SPAN + + +def shuffled(items: list, seed: int, key: Callable[[Any], str]) -> list: + """`items` in the seed's md5 order — the order `ORDER BY md5(key || seed)` gives.""" + return sorted(items, key=lambda it: hashlib.md5(f"{key(it)}{seed}".encode()).hexdigest()) + + +def leaning( + items: list, seed: int, key: Callable[[Any], str], weight: Callable[[Any], float] +) -> list: + """`items` in the weighted draw's order (`weighted_key` in Python), ties in the md5 order.""" + order = {key(it): i for i, it in enumerate(shuffled(items, seed, key))} + return sorted( + items, + key=lambda it: (-math.log(1.0 - u_of(key(it), seed)) / weight(it), order[key(it)]), + ) + + def blend( weighted: list, blind: list, offset: int, n: int, key: Callable[[Any], Hashable] = lambda x: x ) -> list: diff --git a/backend/app/feedquery.py b/backend/app/feedquery.py index 173c806e..5fec78f6 100644 --- a/backend/app/feedquery.py +++ b/backend/app/feedquery.py @@ -13,7 +13,19 @@ import binascii import json from datetime import date, datetime, timedelta, timezone -from sqlalchemy import BigInteger, Select, and_, case, cast, false, func, or_, select +from sqlalchemy import ( + BigInteger, + Select, + and_, + case, + cast, + false, + func, + literal, + or_, + select, + tuple_, +) from sqlalchemy.orm import Session, aliased from app.collation import collated @@ -536,15 +548,17 @@ def sort_keys(sort: str, seed: int, rank_expr=None, lang: str | None = None) -> }, pub, ], - # Deterministic per-seed shuffle: the same seed yields the same total order, so - # the cursor stays valid across pages of one shuffled run. + # The ring of stored random keys from the seed's point (`shuffle_walk`): the keys at or after + # it, then the ones before. The same seed yields the same total order, so the cursor stays + # valid across pages; `feed_page` reads it as two index ranges, never as this sort. "shuffle": [ { - "expr": func.md5(func.concat(Video.id, str(seed))), + "expr": case((Video.shuffle_key < shuffle_start(seed), 1), else_=0), "asc": True, "nullable": False, - "kind": "str", - } + "kind": "num", + }, + {"expr": Video.shuffle_key, "asc": True, "nullable": False, "kind": "num"}, ], } # Relevance (search-result ranking) only exists when there's a query to rank against; @@ -700,6 +714,40 @@ class BadCursor(ValueError): """A `cursor` this module did not write (or one from another sort): the caller's bad input.""" +# The golden ratio's fraction: consecutive seeds land far apart on the ring, never next to each other. +_GOLDEN = 0.6180339887498949 + + +def shuffle_start(seed: int) -> float: + """The point in [0, 1) where a seed's shuffle starts on the ring of `Video.shuffle_key`.""" + return (seed * _GOLDEN) % 1.0 + + +def shuffle_walk( + db: Session, query: Select, seed: int, n: int, after: tuple[int, float, str] | None = None +) -> list: + """Up to `n` rows of `query` in the seed's ring order: shuffle_key from the seed's point upwards, + then from 0 up to that point. `after` is a cursor's (phase, key, id): the walk resumes past it. + + Each arc is an ascending range read along ix_videos_shuffle_key with the scope's filters checked + row by row, so a draw costs the rows it reads, not a sort of the scope — the md5(id || seed) order + it replaced read and sorted every row the filters let through (~350 ms on prod for ~213k).""" + start = shuffle_start(seed) + phase_from, key, last_id = after if after is not None else (0, None, None) + rows: list = [] + for phase in (0, 1): + if phase < phase_from or len(rows) >= n: + continue + arc = Video.shuffle_key >= start if phase == 0 else Video.shuffle_key < start + q = query.where(arc) + if key is not None and phase == phase_from: + q = q.where( + tuple_(Video.shuffle_key, Video.id) > tuple_(literal(key), literal(last_id)) + ) + rows += db.execute(q.order_by(Video.shuffle_key, Video.id).limit(n - len(rows))).all() + return rows + + def feed_page( db: Session, user: User, @@ -734,6 +782,14 @@ def feed_page( k["value"] = v if ranked and window is not None: rows = windowed_relevance_rows(db, query, keys, window, cursor_id, limit + 1) + elif sort == "shuffle": + after = None + if vals is not None and cursor_id is not None: + try: + after = (int(vals[0]), float(vals[1]), str(cursor_id)) + except (TypeError, ValueError) as e: + raise BadCursor("Invalid feed cursor") from e + rows = shuffle_walk(db, query, seed, limit + 1, after) else: query = query.order_by(*order_by(keys)) if cursor_id is not None: diff --git a/backend/tests/test_shuffle_walk.py b/backend/tests/test_shuffle_walk.py new file mode 100644 index 00000000..6c2b70a1 --- /dev/null +++ b/backend/tests/test_shuffle_walk.py @@ -0,0 +1,151 @@ +"""A seeded shuffle walks a stored random key instead of sorting the whole scope. DB lane. + +Every video carries a random number drawn once (`Video.shuffle_key`). A seed picks a point on that +ring; the feed's Shuffle sort pages along the ring from there and wraps, and the Home surprise reads +a window of it and shuffles that window by the seed (`discover.queries.yt_surprise`). Ordering the +scope by md5(id || seed) cost ~350 ms per call on prod and grew with every Shuffle page. +""" + +from contextlib import contextmanager +from datetime import datetime, timedelta, timezone + +from sqlalchemy import event + +from app import feedquery +from app.catalog.models import Channel, Subscription, Video +from app.discover import queries +from app.discover.queries import _FEED_BASE + +NOW = datetime.now(timezone.utc) +KEYS = {"k05": 0.05, "k20": 0.20, "k40": 0.40, "k60": 0.60, "k80": 0.80, "k95": 0.95} + + +def _seed(db, make_user, keys=KEYS): + user = make_user() + db.add(Channel(id="UCring", title="Ring")) + db.flush() + db.add(Subscription(user_id=user.id, channel_id="UCring")) + db.add_all( + [ + Video( + id=vid, + channel_id="UCring", + title=vid, + duration_seconds=20 * 60, + published_at=NOW - timedelta(days=i + 1), + shuffle_key=key, + ) + for i, (vid, key) in enumerate(keys.items()) + ] + ) + db.commit() + return user + + +def _ring(seed, keys=KEYS): + """The ring order from the seed's point: the keys at or after it ascending, then the rest.""" + r = feedquery.shuffle_start(seed) + return sorted(keys, key=lambda v: (keys[v] < r, keys[v])) + + +def _seed_landing_between(lo, hi): + return next(s for s in range(1000) if lo < feedquery.shuffle_start(s) < hi) + + +def test_the_walk_starts_at_the_seeds_point_and_wraps(db, make_user): + user = _seed(db, make_user) + query, _ = feedquery.filtered_query(db, user, **_FEED_BASE) + seed = _seed_landing_between(0.45, 0.55) # starts at k60, wraps to k05 after k95 + got = [r.id for r in feedquery.shuffle_walk(db, query, seed, 6)] + assert got == ["k60", "k80", "k95", "k05", "k20", "k40"] + assert [r.id for r in feedquery.shuffle_walk(db, query, seed, 4)] == got[:4] + + +def test_the_feed_shuffle_pages_along_the_ring(db, make_user, as_user): + user = _seed(db, make_user) + client = as_user(user) + for seed in (_seed_landing_between(0.1, 0.15), _seed_landing_between(0.85, 0.9)): + walked, cursor = [], None + for _ in range(5): + url = f"/api/feed?sort=shuffle&seed={seed}&limit=2" + ( + f"&cursor={cursor}" if cursor else "" + ) + body = client.get(url).json() + walked += [it["id"] for it in body["items"]] + cursor = body["next_cursor"] + if not body["has_more"]: + break + assert walked == _ring(seed), seed + + +def test_a_new_seed_opens_the_shuffle_elsewhere(db, make_user, as_user): + user = _seed(db, make_user) + client = as_user(user) + first = { + seed: client.get(f"/api/feed?sort=shuffle&seed={seed}&limit=2").json()["items"][0]["id"] + for seed in (_seed_landing_between(0.1, 0.15), _seed_landing_between(0.65, 0.7)) + } + assert sorted(first.values()) == ["k20", "k80"] + + +@contextmanager +def _statements(db): + seen: list[str] = [] + + def grab(conn, cursor, statement, *args): + seen.append(statement) + + engine = db.get_bind() + event.listen(engine, "before_cursor_execute", grab) + try: + yield seen + finally: + event.remove(engine, "before_cursor_execute", grab) + + +def _draws_of_videos(seen): + return [s for s in seen if "FROM videos" in s and "LIMIT" in s] + + +def test_neither_draw_orders_the_scope_by_a_hash(db, make_user): + # Each draw reads the videos as an ascending range of the key (what ix_videos_shuffle_key serves), + # never ordered by a hash or by the ring's CASE (which no index can serve). + user = _seed(db, make_user) + with _statements(db) as surprise: + queries.yt_surprise(db, user, "any", 7, 3) + with _statements(db) as page: + feedquery.feed_page( + db, user, dict(_FEED_BASE), sort="shuffle", seed=7, limit=2, cursor=None, lang="en" + ) + for seen in (surprise, page): + draws = _draws_of_videos(seen) + assert draws + for s in draws: + assert "md5(" not in s.lower() + assert "ORDER BY videos.shuffle_key, videos.id" in s.replace("\n", " "), s + + +def test_the_surprise_reads_a_window_of_the_ring(db, make_user, monkeypatch): + user = _seed(db, make_user) + monkeypatch.setattr(queries, "SURPRISE_WINDOW", 3) + for seed in range(20): + drawn = {r["id"] for r in queries.yt_surprise(db, user, "any", seed, 3)} + assert drawn == set(_ring(seed)[:3]), seed + + +def test_a_wide_gap_on_the_ring_does_not_make_a_favourite(db, make_user): + # One video after an empty half of the ring, twenty packed into the other half. Taking the first + # video after the seed's point would deal the lonely one about half the time; the window is + # shuffled by the seed, so it leads about as often as any other. + keys = {"lonely": 0.5} | {f"p{i:02}": 0.51 + i * 0.02 for i in range(20)} + user = _seed(db, make_user, keys) + leads = [queries.yt_surprise(db, user, "any", s, 1)[0]["id"] for s in range(90)] + assert leads.count("lonely") < 15, leads.count("lonely") + + +def test_a_new_video_gets_its_own_key(db, make_user): + _seed(db, make_user, {}) + db.add_all([Video(id=f"n{i}", channel_id="UCring", title="n") for i in range(3)]) + db.commit() + keys = [db.get(Video, f"n{i}").shuffle_key for i in range(3)] + assert len(set(keys)) == 3 and all(0 <= k < 1 for k in keys) From c0682aa572809c608cb9e22f88916e44cc9f1d58 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 7 Nov 2026 15:09:22 +0100 Subject: [PATCH 07/23] fix(plex): a link too slow for the stream resumes in place, and says so A request hls.js got no answer to in time (no status) was taken for a server away: a new session, the same first segment lost again, a restart every half minute that never ended, since every session answer reset the away window. Measured on prod: a work network at ~4 Mbit/s against 11-16 MB remux segments. A timeout now resumes loading in the same session; the away window ends only when media arrives (or a direct stream answers), and runs out in a 'too slow' error. A segment gets 30 s to its first byte. After 5 s of stalling on a link measured slower than the stream, the player says so with both rates. --- frontend/src/i18n/locales/en/plex.json | 3 + frontend/src/i18n/locales/hu/plex.json | 3 + frontend/src/lib/format.test.ts | 9 ++ frontend/src/lib/format.ts | 14 +++ frontend/src/lib/hlsEngine.test.ts | 110 +++++++++++++++++ frontend/src/lib/hlsEngine.ts | 57 ++++++++- frontend/src/lib/hlsPort.test.ts | 112 +++++++++++++++--- frontend/src/lib/hlsPort.ts | 30 ++++- .../lib/plexPlayerHooks.slowLink.dom.test.ts | 60 ++++++++++ frontend/src/lib/plexPlayerHooks.ts | 44 +++++++ frontend/src/lib/plexStats.test.ts | 32 +++++ frontend/src/lib/plexStats.ts | 22 ++++ frontend/src/modules/plex/PlexPlayer.tsx | 29 ++++- 13 files changed, 495 insertions(+), 30 deletions(-) create mode 100644 frontend/src/lib/plexPlayerHooks.slowLink.dom.test.ts diff --git a/frontend/src/i18n/locales/en/plex.json b/frontend/src/i18n/locales/en/plex.json index cbbe8a75..72ff1ecf 100644 --- a/frontend/src/i18n/locales/en/plex.json +++ b/frontend/src/i18n/locales/en/plex.json @@ -191,6 +191,9 @@ "errNotFound": "This file can't be played — the media isn't reachable on the server (the Plex media mount is missing or the path map is wrong). Ask the admin to check the Plex configuration.", "errTranscode": "This file's format needs transcoding, which isn't supported yet.", "errGeneric": "Playback couldn't start. Please try again.", + "errSlow": "Couldn't play: the connection is too slow for this file", + "slowLink": "Slow connection", + "slowLinkRates": "This file needs about {{need}}, about {{got}} is arriving", "help": "Keyboard shortcuts", "infoBtn": "Media info", "more": "More", diff --git a/frontend/src/i18n/locales/hu/plex.json b/frontend/src/i18n/locales/hu/plex.json index bbf6264b..a7e17d40 100644 --- a/frontend/src/i18n/locales/hu/plex.json +++ b/frontend/src/i18n/locales/hu/plex.json @@ -191,6 +191,9 @@ "errNotFound": "Ez a fájl nem játszható le — a média nem érhető el a szerveren (hiányzik a Plex media-mount, vagy hibás az útvonal-leképezés). Kérd meg az adminisztrátort, hogy ellenőrizze a Plex beállításait.", "errTranscode": "Ennek a fájlnak a formátuma átkódolást igényel, ami még nem támogatott.", "errGeneric": "Nem sikerült elindítani a lejátszást. Próbáld újra.", + "errSlow": "Nem sikerült lejátszani: a kapcsolat túl lassú ehhez a fájlhoz", + "slowLink": "Lassú a kapcsolat", + "slowLinkRates": "A fájlhoz kb. {{need}} kell, most kb. {{got}} érkezik", "help": "Billentyűparancsok", "infoBtn": "Média infó", "more": "Továbbiak", diff --git a/frontend/src/lib/format.test.ts b/frontend/src/lib/format.test.ts index 096bb752..5d075edd 100644 --- a/frontend/src/lib/format.test.ts +++ b/frontend/src/lib/format.test.ts @@ -6,6 +6,7 @@ import { formatCompact, formatCountOrDash, formatDecimal, + formatMbps, formatMoney, formatDate, formatDateTime, @@ -284,3 +285,11 @@ describe("numbers in the UI language", () => { expect(formatMoney(160_000_000, "hu")).not.toBe(formatMoney(160_000_000, "en")); }); }); + +describe("formatMbps", () => { + it("writes a rate in the browser's own unit, a decimal only under 10", () => { + expect(formatMbps(19_600_000, "en")).toBe("20 Mb/s"); + expect(formatMbps(4_130_000, "en")).toBe("4.1 Mb/s"); + expect(formatMbps(4_130_000, "hu")).toBe("4,1 Mb/mp"); + }); +}); diff --git a/frontend/src/lib/format.ts b/frontend/src/lib/format.ts index 9fe40ca6..519e9bf8 100644 --- a/frontend/src/lib/format.ts +++ b/frontend/src/lib/format.ts @@ -133,6 +133,20 @@ export function formatUnit( ); } +/** A rate in megabits per second, in the browser's own short form ("20 Mb/s" / "20 Mb/mp"): one + * decimal under 10, whole above. */ +export function formatMbps(bitsPerSecond: number, lang: string): string { + const mbps = bitsPerSecond / 1_000_000; + const digits = mbps < 10 ? 1 : 0; + return numberFormat(lang, { + style: "unit", + unit: "megabit-per-second", + unitDisplay: "short", + maximumFractionDigits: digits, + minimumFractionDigits: digits, + }).format(mbps); +} + /** Compact human runtime ("1 hr 30 min", "45 min", "45 sec"; HU "1 ó 30 p") from seconds. Formats * BOTH a single item's length (Plex movies/episodes) and an aggregate total (playlist playtime, on * the YouTube and the Plex side alike) — it is the one renderer for a playlist total, so a total diff --git a/frontend/src/lib/hlsEngine.test.ts b/frontend/src/lib/hlsEngine.test.ts index 38c54da3..88f52781 100644 --- a/frontend/src/lib/hlsEngine.test.ts +++ b/frontend/src/lib/hlsEngine.test.ts @@ -46,6 +46,7 @@ class FakeHls implements HlsPort { loaded: string | null = null; destroyed = false; audioTrack = 0; + resumes = 0; constructor(readonly handlers: HlsHandlers) {} load(url: string) { this.loaded = url; @@ -53,6 +54,24 @@ class FakeHls implements HlsPort { destroy() { this.destroyed = true; } + resume() { + this.resumes++; + } + /** hls.js gave up on a request that never answered in time: no response, so no status. */ + timedOut(details = "fragLoadTimeOut") { + this.handlers.onError({ + fatal: true, + network: true, + status: null, + timeout: true, + type: "networkError", + details, + }); + } + /** A fragment arrived: media is getting through. */ + arrived() { + this.handlers.onFragLoaded({ type: "main", bytes: 1_000_000, durationS: 6 }); + } getAudioTrack() { return this.audioTrack; } @@ -72,6 +91,7 @@ class FakeHls implements HlsPort { fatal: false, network: true, status: 410, + timeout: false, type: "networkError", details: "fragLoadError", }); @@ -82,6 +102,7 @@ class FakeHls implements HlsPort { fatal: false, network: true, status, + timeout: false, type: "networkError", details: "fragLoadError", }); @@ -92,6 +113,7 @@ class FakeHls implements HlsPort { fatal: true, network: true, status, + timeout: false, type: "networkError", details: "levelLoadError", }); @@ -579,6 +601,7 @@ describe("a server that is restarting (a deploy)", () => { fire(); await asks[1]!.resolve(hls("A")); hlses[1]!.handlers.onManifestParsed(); + hlses[1]!.arrived(); // back: media gets through again advance(AWAY_WINDOW_MS + 1); hlses[1]!.lost(502); // a second deploy, long after the first expect(engine.getSnapshot().error).toBeNull(); @@ -608,6 +631,93 @@ describe("a server that is restarting (a deploy)", () => { hlses[0]!.lost(500); expect(engine.getSnapshot().error).toBe("generic"); }); + + it("takes a direct stream's answer for the server being back: it has no fragments to show it", async () => { + const { engine, asks, fire, advance } = setup(); + engine.start("A", 0); + await asks[0]!.reject({ status: 502 }); + fire(); + await asks[1]!.resolve(direct("A")); + advance(AWAY_WINDOW_MS + 1); + engine.start("A", 100); // a second deploy, long after the first + await asks[2]!.reject({ status: 502 }); + expect(engine.getSnapshot().error).toBeNull(); + }); + + it("ends in an error when the server answers every session but none of them delivers", async () => { + // Every answer used to start the absence over, so this asked for a new session forever. + const { engine, asks, hlses, fire } = setup(); + engine.start("A", 0); + await asks[0]!.resolve(hls("A")); + hlses[0]!.handlers.onManifestParsed(); + hlses[0]!.lost(0); + for (let i = 1; i < 40 && fire(); i++) { + await asks[i]!.resolve(hls("A", 0, `s-${i}`)); + hlses[i]!.handlers.onManifestParsed(); + hlses[i]!.lost(0); + } + expect(engine.getSnapshot().error).toBe("generic"); + expect(asks.length).toBeLessThan(40); + }); +}); + +describe("a link too slow for the stream", () => { + // Measured on prod (2026-11-04, a work network at ~4 Mbit/s against 11–16 MB remux segments): an + // audio segment waited behind a video one past hls.js's time to first byte, five times, and gave up + // with no answer. That was taken for a server away: a new session, the same segment lost again, a + // restart every half minute until the viewer closed the player. + it("resumes loading in the same session, and asks for no new one", async () => { + const { engine, asks, hlses, timers } = setup(); + engine.start("A", 453); + await asks[0]!.resolve(hls("A", 453)); + hlses[0]!.handlers.onManifestParsed(); + hlses[0]!.timedOut(); + expect(asks).toHaveLength(1); + expect(timers.filter((t) => t.armed)).toHaveLength(0); + expect(hlses[0]!.resumes).toBe(1); + expect(hlses[0]!.destroyed).toBe(false); + expect(engine.getSnapshot()).toMatchObject({ ready: true, error: null }); + }); + + it.each(["fragLoadTimeOut", "levelLoadTimeOut", "audioTrackLoadTimeOut"])( + "treats a %s the same way", + async (details) => { + const { engine, asks, hlses } = setup(); + engine.start("A", 0); + await asks[0]!.resolve(hls("A")); + hlses[0]!.timedOut(details); + expect(asks).toHaveLength(1); + expect(hlses[0]!.resumes).toBe(1); + }, + ); + + it("keeps going for as long as media still arrives, however slowly", async () => { + const { engine, asks, hlses, advance } = setup(); + engine.start("A", 0); + await asks[0]!.resolve(hls("A")); + hlses[0]!.handlers.onManifestParsed(); + for (let i = 0; i < 10; i++) { + hlses[0]!.timedOut(); + advance(60_000); + hlses[0]!.arrived(); + } + hlses[0]!.timedOut(); + expect(engine.getSnapshot().error).toBeNull(); + expect(asks).toHaveLength(1); + }); + + it("says the connection is too slow once nothing has arrived for the whole window", async () => { + const { engine, asks, hlses, advance } = setup(); + engine.start("A", 0); + await asks[0]!.resolve(hls("A")); + hlses[0]!.handlers.onManifestParsed(); + hlses[0]!.timedOut(); + advance(AWAY_WINDOW_MS + 1); + hlses[0]!.timedOut(); + expect(engine.getSnapshot().error).toBe("slow"); + expect(hlses[0]!.resumes).toBe(1); + expect(asks).toHaveLength(1); + }); }); describe("play intent while a session is asked for", () => { diff --git a/frontend/src/lib/hlsEngine.ts b/frontend/src/lib/hlsEngine.ts index 4436c63a..7fa75875 100644 --- a/frontend/src/lib/hlsEngine.ts +++ b/frontend/src/lib/hlsEngine.ts @@ -30,14 +30,20 @@ export const RECLAIM_WINDOW_MS = 120_000; // gone with it, but the buffer keeps playing — so the engine asks again where the viewer is, less and // less often, and says it cannot play only once the server has been away this long. It used to say // so at the first 502, over a picture still playing from the buffer, and never asked again. +// +// The absence ends when MEDIA arrives again (a fragment loads), not when a session request is +// answered: a server that answers every session and delivers none of them is still not serving, and +// ending the absence at each answer restarted the stream forever. The same window bounds a link too +// slow for the stream (see `slowLink`). export const AWAY_WINDOW_MS = 180_000; const AWAY_RETRY_MS = [2_000, 4_000, 8_000, 15_000]; const SERVER_AWAY = new Set([0, 502, 503, 504]); // The server's answer for a session it no longer has; its 404 means a segment not there yet. const SESSION_GONE = 410; -/** Why a session could not play, in the three buckets the player has a sentence for. */ -type EngineError = "transcode" | "notFound" | "generic"; +/** Why a session could not play, in the buckets the player has a sentence for. `slow`: the link + * never delivered the stream in time for the whole away window. */ +type EngineError = "transcode" | "notFound" | "slow" | "generic"; /** What the player renders from the engine. A new object whenever any of it changes. */ export interface EngineSnapshot { @@ -55,14 +61,27 @@ export interface HlsFailure { fatal: boolean; network: boolean; status: number | null; + /** A request that got no answer in time (no response, so `status` is null). */ + timeout: boolean; type: string; details: string; } +/** A loaded fragment, as the meters read it. `loading`: when its download started and ended + * (performance.now() ms), when known. */ +export interface FragLoaded { + type: string; + bytes: number; + durationS: number; + loading?: { start: number; end: number }; +} + /** One hls.js instance, narrowed to what the engine uses (hlsPort.ts binds the real one). */ export interface HlsPort { load(url: string, media: HTMLMediaElement): void; destroy(): void; + /** Start loading again where it stopped, after a fatal network error (same session). */ + resume(): void; getAudioTrack(): number; setAudioTrack(track: number): void; levelInfo(): HlsLevelInfo; @@ -71,7 +90,7 @@ export interface HlsPort { export interface HlsHandlers { onManifestParsed(): void; onAudioTracksUpdated(): void; - onFragLoaded(frag: { type: string; bytes: number; durationS: number }): void; + onFragLoaded(frag: FragLoaded): void; onError(e: HlsFailure): void; } @@ -106,7 +125,7 @@ export interface EngineDeps { statusOf(e: unknown): number | undefined; /** A request for a session went out: the player clears its stall spinner. */ onLoadStart?(): void; - onFragLoaded?(frag: { type: string; bytes: number; durationS: number }): void; + onFragLoaded?(frag: FragLoaded): void; onHlsError?(e: HlsFailure): void; } @@ -358,7 +377,9 @@ export class HlsEngine { } const media = this.deps.media(); if (!media) return; - this.away = null; // the server answered: a later absence is a new one + // A direct stream reports no fragments: its answer is the only sign the server is back. An HLS + // absence ends when a fragment arrives (see `AWAY_WINDOW_MS`). + if (sess.mode !== "hls") this.away = null; const { intent, startAt, id } = req; // The stream that kept playing is replaced from here: the new one loads like any other. if (req.quiet) this.set({ ready: false }); @@ -456,7 +477,10 @@ export class HlsEngine { /* out of range — ignore */ } }, - onFragLoaded: (frag) => this.deps.onFragLoaded?.(frag), + onFragLoaded: (frag) => { + if (this.current(req)) this.away = null; // media gets through: whatever was away is back + this.deps.onFragLoaded?.(frag); + }, onError: (e) => this.onHlsError(req, media, e), }; } @@ -489,6 +513,10 @@ export class HlsEngine { return; } if (!e.fatal) return; + if (e.network && e.timeout) { + this.slowLink(); + return; + } if (e.network && SERVER_AWAY.has(e.status ?? 0) && this.serverAway(req)) return; if (e.network && e.status === 404) { this.reclaim(req, media, false); @@ -538,6 +566,23 @@ export class HlsEngine { return true; } + /** A request got no answer in time: the link is too slow for what the stream asks of it, not the + * server away (measured on prod: a work network at ~4 Mbit/s against 11–16 MB remux segments; an + * audio segment waited behind a video one past hls.js's time to first byte). A new session would + * throw away what had arrived and lose its first segment the same way — a restart every half + * minute, never playing. So loading resumes in the same session, for as long as media still + * arrives; the away window says it is too slow once nothing has for that long. */ + private slowLink(): void { + const now = this.deps.now(); + const away = (this.away ??= { since: now, tries: 0 }); + if (now - away.since > AWAY_WINDOW_MS) { + this.set({ error: "slow" }); + return; + } + away.tries += 1; + this.hls?.resume(); + } + /** Where the viewer is: the ticked position, or — before playback has ticked (a session still * loading, or a resumed one that has stayed paused) — where the session was ASKED to start. * Anything that restarts "here" must use this: reading the ticked 0 restarted a resume at 20:00 diff --git a/frontend/src/lib/hlsPort.test.ts b/frontend/src/lib/hlsPort.test.ts index aea69cc0..e26e8a1a 100644 --- a/frontend/src/lib/hlsPort.test.ts +++ b/frontend/src/lib/hlsPort.test.ts @@ -1,42 +1,124 @@ +// The one place hls.js is built for the Plex player, held against a stand-in hls.js: what config it is +// built with, and what it hands the engine from hls.js's events. +// // hls.js empties the cues of every text track on the