diff --git a/CHANGELOG.md b/CHANGELOG.md index 994a5912..bca46e62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,26 @@ edit the release notes and re-run the generator. Upgrading, pinning a version and rolling back: [docs/self-hosting.md](docs/self-hosting.md#upgrading). +## 0.83.0 — 2026-09-09 + +Siftlode now opens on a page that answers one question: what should I watch right now. You say how much time you have — ten minutes, half an hour, a whole evening — and it gives you one suggestion, from YouTube or from Plex, with a button that picks up where you left off. Under it sits a shelf of the things you always come back to, which you fill yourself. + +### Features + +- A new Home page, and it is where Siftlode starts. It asks how much time you have — 10 minutes, 30 minutes, an evening, or it does not matter — and answers with ONE suggestion rather than a wall of thumbnails. The question is on top, the answer directly under it. If you would rather land on the feed as before, Settings → Appearance → Startup turns it off. +- The time you have picks the source, not just the length. Ten minutes gets you a short video; an evening is allowed to offer a film. It matches on the time you have LEFT in something, so a series episode you are four minutes from finishing counts as a four-minute answer. +- The suggestion is one of three things, and it says which: something you left half-finished, something off your comfort shelf, or a surprise. Continue takes you back to the second you stopped. Something else rerolls it — there is no separate surprise-me page, the reroll is it. +- A comfort shelf you fill yourself, for the media you put on without thinking. It takes a whole channel, a whole show, a film, a single video or a playlist — because comfort viewing is put on TheVR, not play this one episode. One click resolves it: a show gives you the next unwatched episode, a channel its newest unwatched video. The shelf can be reordered into the order you want. +- Pin from wherever you are: the same ⋯ menu — or a right-click — on a feed video, a channel row, a playlist, a Plex poster or a Plex row now carries a Comfort shelf entry. Feed video cards answer a right-click for the first time. +- Continue watching, on both sides at once. The rail merges what you left half-watched on YouTube with what you left half-watched on Plex, most recently touched first — the YouTube half of that did not exist before. +- Shift+W jumps to Home from anywhere, including out of a player, and it is in the keyboard sheet. The jump-to box (Ctrl+K) gained a Surprise me row, and typing what should I watch finds Home itself. + +### Internal + +- E24, four sprints. S1 built a discover read-model as its own backend package — the unified card shape maps the EXISTING Plex and feed serializers rather than adding a base serializer, the rails reuse the feed query layer and the Plex rollup, and the tiered hero rule is one shared function that returns the tier it chose, so the UI prints an honest kicker instead of guessing. The shelf is a typed comfort_pins table with a one-of check constraint and a real foreign key per kind, not a polymorphic string reference: a deleted Plex item must not leave a dead pin on the shelf. S2 built the page on the existing carousel and the two existing card components rather than a third. S3 wired pinning through the context-menu input path E10 shipped, which absorbed that epic's parked video-card item. S4 added the ways in. +- The closing review ran four rounds, 46 → 23 → 19 → 15 → 14, each round reviewing the previous round's own fix diff, with three adversarial verifiers per finding in rounds one to three. Four of round one's findings died at 2-of-3 and are recorded so they are not raised again. What the eight-lane gate could not have caught: the resume rail built its own query and dropped the per-user channel blocklist, so a video from a channel you had just banned could become the landing page's suggestion; the shelf numbered a new pin by row COUNT while nothing renumbers, so after any non-tail unpin a pin landed mid-shelf and could collide; the Home tile and the show page disagreed about which episode comes next, one taking the first in-progress episode and the other the last; and Shift+W was dead whenever anything was open, because the page change unmounted the player and its cleanup reclaimed the history entry the navigation had just pushed. Rounds three and four stopped finding defects in the epic and started finding them in the review's own fix commits — a test named after a branch it never reached, comments claiming more than the code does — which is the signal to stop. +- Two process notes worth keeping. A commit in this epic fixed a bug that does not exist: its message and two comments claimed a sort key raises when two rows both lack a timestamp, which tuple comparison never reaches — three verifiers executed it independently, and the rationale is now corrected in place rather than quietly deleted. And three of the regression tests written during the fix rounds could not go red for the defect they named until they were rewritten, which is why every test added in a fix round on this epic was mutation-checked. + ## 0.82.0 — 2026-09-06 Siftlode gets a keyboard. There is one place that lists every shortcut, a jump-to box that takes you anywhere by name, and a right-click menu on playlist rows — plus the whole app now holds together at a tablet's width and stops thinning itself out on a very wide screen. diff --git a/VERSION b/VERSION index 92fc430a..83dcd12c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.82.0 +0.83.0 diff --git a/backend/alembic/versions/0083_comfort_pins.py b/backend/alembic/versions/0083_comfort_pins.py new file mode 100644 index 00000000..e4774457 --- /dev/null +++ b/backend/alembic/versions/0083_comfort_pins.py @@ -0,0 +1,78 @@ +"""comfort_pins — the shelf you fall back to when you cannot decide + +Revision ID: 0083_comfort_pins +Revises: 0082_invite_lang +Create Date: 2026-09-06 + +E24's one new table. The shelf holds five different things — a video, a YouTube channel, a Plex show, +a Plex movie, a playlist — because comfort media is usually "put on X", a series or a channel, and +almost never one particular video. + +Three shapes were possible and two are wrong here: + +- a boolean on `video_states` / `plex_states` cannot ORDER a shelf, and has nowhere to put a channel + or a show (neither has a per-user state row at all); +- a polymorphic `(kind, ref)` string pair orders and holds anything, but drops the foreign key — and a + Plex item removed from the library, or a channel the user unsubscribes and prunes, would then leave + a dead tile on the one surface whose entire job is to be reliable when nothing else is. + +So: one nullable CASCADE-ing FK per kind, plus `num_nonnulls(...) = 1`. The union invariant lives in +the schema rather than in a comment at every insert site, which is the same rule that the +`cancelled`/`canceled` bug taught this codebase. + +NULLs are DISTINCT in a Postgres unique index, so the five per-kind `(user_id, )` uniques each +stop a double-pin without colliding with the other four kinds' NULL rows. + +No backfill: an empty shelf is the honest state for every existing account. The surface renders its +own empty state and invites the first pin. +""" + +import sqlalchemy as sa +from alembic import op + +revision: str = "0083_comfort_pins" +down_revision: str | None = "0082_invite_lang" +branch_labels: str | None = None +depends_on: str | None = None + + +def upgrade() -> None: + op.create_table( + "comfort_pins", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column( + "user_id", + sa.Integer(), + sa.ForeignKey("users.id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column("video_id", sa.String(length=16), sa.ForeignKey("videos.id", ondelete="CASCADE")), + sa.Column( + "channel_id", sa.String(length=32), sa.ForeignKey("channels.id", ondelete="CASCADE") + ), + sa.Column("plex_item_id", sa.Integer(), sa.ForeignKey("plex_items.id", ondelete="CASCADE")), + sa.Column("plex_show_id", sa.Integer(), sa.ForeignKey("plex_shows.id", ondelete="CASCADE")), + sa.Column("playlist_id", sa.Integer(), sa.ForeignKey("playlists.id", ondelete="CASCADE")), + sa.Column("position", sa.Integer(), nullable=False, server_default="0"), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.func.now(), + nullable=False, + ), + sa.CheckConstraint( + "num_nonnulls(video_id, channel_id, plex_item_id, plex_show_id, playlist_id) = 1", + name="ck_comfort_pin_one_ref", + ), + sa.UniqueConstraint("user_id", "video_id", name="uq_comfort_pin_video"), + sa.UniqueConstraint("user_id", "channel_id", name="uq_comfort_pin_channel"), + sa.UniqueConstraint("user_id", "plex_item_id", name="uq_comfort_pin_plex_item"), + sa.UniqueConstraint("user_id", "plex_show_id", name="uq_comfort_pin_plex_show"), + sa.UniqueConstraint("user_id", "playlist_id", name="uq_comfort_pin_playlist"), + ) + op.create_index("ix_comfort_pins_user_id", "comfort_pins", ["user_id"]) + + +def downgrade() -> None: + op.drop_index("ix_comfort_pins_user_id", table_name="comfort_pins") + op.drop_table("comfort_pins") diff --git a/backend/app/discover/__init__.py b/backend/app/discover/__init__.py new file mode 100644 index 00000000..e6f779b7 --- /dev/null +++ b/backend/app/discover/__init__.py @@ -0,0 +1,10 @@ +"""E24 — the read-model behind the Home surface ("Mit nézzek most?"). + +The anti-paralysis answer layer: one hero suggestion, a cross-source resume rail, and the user's own +comfort shelf, all served from our own Postgres with no upstream call at serve time (the same rule +the Plex Discover surface follows). + +Layering, mirroring `app/plex/`: `budget` (the time bands) and `cards` (the unified row shape) are +pure; `queries` holds the SQL; `rails`, `pins` and `hero` compose. Nothing here imports from +`app.routes` — `routes/discover.py` is a thin HTTP shell over these. +""" diff --git a/backend/app/discover/budget.py b/backend/app/discover/budget.py new file mode 100644 index 00000000..23e2f558 --- /dev/null +++ b/backend/app/discover/budget.py @@ -0,0 +1,62 @@ +"""The time budget — E24's replacement for "what mood are you in?". + +`random-gondolatok` 15/D asked for a mood questionnaire before the recommendations. The same file's +later BA-interview note (#23) says the mood half is not what makes this work: the friction is +deciding at all. A mood→genre mapping would also be invented data — we store no mood signal and never +will — whereas "how much time do you have" is a question the library can actually answer, from a +column we already have. + +So the surface asks about TIME, and the answer is deterministic: four bands, no model, no latency. +The band also picks the source for free — ten minutes is a YouTube video, an evening is a movie — +which is exactly the cross-source behaviour the Home surface exists to provide. + +The bands overlap on purpose. "I have 30 minutes" should not be answered with a two-minute Short just +because it fits; a lower bound is as much a part of the promise as the upper one. +""" + +from typing import Literal + +Budget = Literal["short", "medium", "long", "any"] + +DEFAULT_BUDGET: Budget = "any" + +# budget -> (min_seconds, max_seconds); None = unbounded on that side. +BANDS: dict[str, tuple[int | None, int | None]] = { + "short": (None, 12 * 60), # "10 perc" — a bit of slack over the round number the chip shows + "medium": (8 * 60, 40 * 60), # "30 perc" + "long": (40 * 60, None), # "Egy este" + "any": (None, None), # "Mindegy" +} + + +def band(budget: str) -> tuple[int | None, int | None]: + """The (min, max) seconds for a budget. Unknown budgets fall back to the unbounded band rather + than raising: the route validates the vocabulary with a Literal, so anything reaching here with a + bad value is our own bug, and an unfiltered shelf is a better failure than a 500 on the landing + page.""" + return BANDS.get(budget, BANDS["any"]) + + +def remaining_seconds(duration_seconds: int | None, position_seconds: int) -> int | None: + """What is actually LEFT to watch. The question is "how much time do you have", so a 90-minute + film with 8 minutes to go belongs in the ten-minute band, not the evening one. Unknown duration + stays unknown — see `fits`.""" + if duration_seconds is None: + return None + return max(0, duration_seconds - max(0, position_seconds)) + + +def fits(seconds: int | None, budget: str) -> bool: + """Does a runtime fit the budget? + + An unknown duration fits only the unbounded band. We cannot promise an item finishes inside ten + minutes when we do not know how long it is, and quietly including it would make the one control + on this page dishonest — the failure mode the house rules call an un-honest state.""" + lo, hi = band(budget) + if seconds is None: + return lo is None and hi is None + if lo is not None and seconds < lo: + return False + if hi is not None and seconds > hi: + return False + return True diff --git a/backend/app/discover/cards.py b/backend/app/discover/cards.py new file mode 100644 index 00000000..eb481364 --- /dev/null +++ b/backend/app/discover/cards.py @@ -0,0 +1,141 @@ +"""The unified discover card: one row shape the Home surface can render without knowing which +module it came from. + +This is deliberately a THIN mapping, not a new serializer layer. The per-source projections already +exist and are already the shapes the frontend's `VideoCard` and `PlexPosterCard` consume — +`feedquery.serialize` for YouTube, `plex.cards.movie_card`/`show_card`/`leaf_card` for Plex — so each +unified card carries the native payload under `video` / `plex` alongside the handful of fields the +rail chrome itself needs (title, thumbnail, progress, runtime). The Home page therefore renders with +the existing card components rather than a third one, and a field added to a feed row or a Plex card +still gets added in exactly one place. CLAUDE.md's "not wanted" list names a base serializer for +precisely this reason. + +`aspect` is the one genuinely new field. A rail has to know whether a tile is a 16:9 still or a 2:3 +poster, and that is a property of the ARTWORK, not of the module: a Plex episode ships a landscape +still exactly like a YouTube video does, while a movie and a show ship posters. Emitting it lets the +rail layer stay domain-free instead of branching on `source`. + +`subtitle` only ever carries DATA — a channel name, a show title, a year. Nothing here composes an +English phrase: this repo is HU+EN with zero key drift, and a sentence assembled in the backend is a +string no translation file can reach. Anything that needs wording (season counts, "N videó") is left +to the client, which has both the native payload and the translations. +""" + +from app.discover.budget import remaining_seconds +from app.models import Channel, Playlist + +# Plex artwork is a poster for the things you browse and a landscape still for the things you play. +_POSTER_KINDS = ("movie", "show") + + +def _base( + *, + source: str, # "youtube" | "plex" + kind: str, # "video" | "movie" | "show" | "episode" | "channel" | "playlist" + id: str, + title: str | None, + aspect: str, # "wide" (16:9) | "poster" (2:3) + subtitle: str | None = None, + thumb: str | None = None, + duration_seconds: int | None = None, + position_seconds: int = 0, + status: str = "new", +) -> dict: + return { + "source": source, + "kind": kind, + "id": id, + "title": title or "", + "subtitle": subtitle, + "thumb": thumb, + "aspect": aspect, + "duration_seconds": duration_seconds, + "position_seconds": position_seconds, + "remaining_seconds": remaining_seconds(duration_seconds, position_seconds), + "status": status, + # Native payloads — at most one is ever set. The client opens the player from these. + "video": None, + "plex": None, + } + + +def key(card: dict) -> str: + """The identity of a card ACROSS sources — `:`. A Plex rating_key and a YouTube video + id could in principle collide as bare strings, and the reroll's exclusion list has to be able to + say "not that one" about either.""" + return f"{card['source']}:{card['id']}" + + +def from_video(row: dict) -> dict: + """`row` is a `feedquery.serialize` dict — the same object the feed grid renders.""" + card = _base( + source="youtube", + kind="video", + id=row["id"], + title=row["title"], + aspect="wide", + subtitle=row.get("channel_title"), + thumb=row.get("thumbnail_url"), + duration_seconds=row.get("duration_seconds"), + position_seconds=row.get("position_seconds") or 0, + status=row.get("status") or "new", + ) + card["video"] = row + return card + + +def from_plex(row: dict) -> dict: + """`row` is a `plex.cards` dict (movie / show / leaf). Its own `type` is the kind.""" + kind = row.get("type") or "movie" + if kind == "episode": + subtitle = row.get("show_title") + elif kind == "show": + subtitle = None # season count lives in the native payload; the client words it + else: + subtitle = str(row["year"]) if row.get("year") else None + + card = _base( + source="plex", + kind=kind, + id=row["id"], + title=row.get("title"), + aspect="poster" if kind in _POSTER_KINDS else "wide", + subtitle=subtitle, + thumb=row.get("thumb"), + duration_seconds=row.get("duration_seconds"), + position_seconds=row.get("position_seconds") or 0, + status=row.get("status") or "new", + ) + card["plex"] = row + return card + + +def from_channel(ch: Channel, unwatched: int) -> dict: + """A pinned channel. It is not itself playable — the shelf resolves it to its newest unwatched + video on click (see `pins._shelf_card`) — so the tile advertises what is waiting there instead of a + runtime it does not have.""" + card = _base( + source="youtube", + kind="channel", + id=ch.id, + title=ch.title, + aspect="wide", + thumb=ch.thumbnail_url, + ) + card["unwatched_count"] = unwatched + return card + + +def from_playlist(pl: Playlist, item_count: int, thumb: str | None) -> dict: + """A pinned playlist. `thumb` is its first item's thumbnail — a shelf of blank tiles would defeat + the point of a surface you are supposed to be able to scan at a glance.""" + card = _base( + source="youtube", + kind="playlist", + id=str(pl.id), + title=pl.name, + aspect="wide", + thumb=thumb, + ) + card["item_count"] = item_count + return card diff --git a/backend/app/discover/hero.py b/backend/app/discover/hero.py new file mode 100644 index 00000000..df7a3320 --- /dev/null +++ b/backend/app/discover/hero.py @@ -0,0 +1,104 @@ +"""The hero: the single suggestion the Home surface leads with. + +One rule, in one function, because the page's entire promise is that it answers rather than offers. +Three tiers, tried in order: + +1. **resume** — something already started whose REMAINING time fits the budget. Almost always the + right answer: it is the thing the user themselves chose most recently, and finishing it costs no + decision at all. +2. **comfort** — a pinned shelf entry that fits. The user's own declared fallback, which beats + anything we could infer. +3. **surprise** — a seeded draw from what is unseen and fits. + +That order is the PAGE's answer. "Mást kérek" rotates it (`start`), so each press offers a different +KIND of answer rather than the next row of the same list — the user's call, 2026-09-06, after the +button was measured cycling nine resume cards and never once reaching the shelf. + +The tier travels with the card so the client can label it honestly ("Ott hagytad abba" vs "A +polcodról" vs "Meglepetés") instead of guessing from the shape of the payload — a kicker that says +"where you left off" over a film the user has never opened is exactly the kind of quiet lie the house +rules call a dishonest state. + +Budgets are matched against REMAINING time throughout (`budget.remaining_seconds`, which equals the +runtime for anything not yet started): with eight minutes left, a 90-minute film is a ten-minute +answer. +""" + +from sqlalchemy.orm import Session + +from app.discover import cards, pins, rails +from app.discover.budget import fits +from app.models import User + +HERO_TIERS = ("resume", "comfort", "surprise") + +# How deep the surprise tier draws before giving up. Deliberately deeper than one, so an excluded +# pick moves the tier along instead of emptying it (the shuffle is seeded — the same seed always +# hands back the same order, so without the headroom "already shown" would mean "nothing left"). +SURPRISE_DRAW = 12 + + +def pick( + db: Session, + user: User, + budget: str, + seed: int, + *, + resume: list[dict] | None = None, + shelf: list[dict] | None = None, + exclude: set[str] | None = None, + start: int = 0, +) -> dict | None: + """`{"tier": ..., "card": ...}`, or None when nothing at all fits. + + `resume` and `shelf` may be passed pre-computed so the consolidated `/api/discover` response does + not build the same two rails twice; the reroll endpoint passes neither and pays only for what it + reaches. + + `exclude` is what makes "Mást kérek" mean something. A new seed alone only re-draws the SURPRISE + tier: the resume tier deterministically returns the most recently progressed thing that fits, so + pressing reroll on a user who has something half-finished handed back the identical card and the + button looked dead (measured in the browser, 2026-09-06). Skipping the cards already shown makes + every press produce a different answer, at every tier. + + `start` ROTATES which tier is tried first — 0 keeps the priority order (the page's own first + answer), 1 leads with comfort, 2 with surprise. Exclusion alone was not enough: the resume rail + holds up to 20 rows, so the button walked down it and never reached the shelf or a surprise at + all (measured, 2026-09-06: twelve presses returned nine cards, every one of them `resume`). The + fallthrough is kept, so a press does not come back empty just because the tier it led with has + nothing to offer.""" + skip = exclude or set() + + def _resume() -> dict | None: + for card in rails.continue_watching(db, user) if resume is None else resume: + playable = rails.playable(db, user.id, card) + # Excluded on the card the USER saw (the show), not only on what it resolves to, so a + # rerolled-past show does not come straight back as its own next episode. + if cards.key(card) in skip or (playable is not None and cards.key(playable) in skip): + continue + if playable is not None and fits(playable["remaining_seconds"], budget): + return playable + return None + + def _comfort() -> dict | None: + for card in pins.playables(pins.shelf(db, user) if shelf is None else shelf): + if cards.key(card) not in skip and fits(card["remaining_seconds"], budget): + return card + return None + + def _surprise() -> dict | None: + # A wider draw than the one card we need: the shuffle is seeded, so without the headroom an + # excluded pick would leave the tier empty rather than move past it. + for card in rails.surprise(db, user, budget, seed, limit=SURPRISE_DRAW): + if cards.key(card) not in skip: + return card + return None + + tiers = {"resume": _resume, "comfort": _comfort, "surprise": _surprise} + offset = start % len(HERO_TIERS) + for tier in HERO_TIERS[offset:] + HERO_TIERS[:offset]: + card = tiers[tier]() + if card is not None: + return {"tier": tier, "card": card} + + return None diff --git a/backend/app/discover/pins.py b/backend/app/discover/pins.py new file mode 100644 index 00000000..aa79ecd8 --- /dev/null +++ b/backend/app/discover/pins.py @@ -0,0 +1,173 @@ +"""The comfort shelf: pin a thing you can always fall back to, and have it be one click from playing. + +Five kinds are pinnable — a video, a YouTube channel, a Plex show, a Plex movie, a playlist — because +comfort media is almost always "put on X", a series or a channel, and only sometimes one particular +video. `models.ComfortPin` carries that union as five nullable FKs under a `num_nonnulls(...) = 1` +check; this module is where the union is spoken about in one vocabulary, so no call site invents a +second one. + +Every shelf card carries a resolved `next`: the thing that actually plays when you click the tile — +a show's in-progress-or-next episode, a channel's newest unwatched video, a playlist's first +unwatched item. It costs a query per non-leaf pin, which is affordable precisely because this shelf is +short and hand-curated, and it buys the property the whole surface exists for: one click, no +intermediate page, no second decision. + +Domain module, so nothing here raises HTTP. `resolve_ref` returns None for "no such thing, or not +yours" and the route turns that into its 404 — core never imports routes. +""" + +from sqlalchemy import func, select +from sqlalchemy.orm import Session + +from app.discover import cards, queries +from app.models import Channel, ComfortPin, PlexItem, PlexShow, Playlist, User, Video +from app.plex import rollup +from app.plex.cards import leaf_card, show_card +from app.plex.lookups import by_rating_key + +# kind -> the ComfortPin column that holds it. The client's vocabulary and the schema's, in one place. +PIN_COLUMNS: dict[str, str] = { + "video": "video_id", + "channel": "channel_id", + "plex_item": "plex_item_id", + "plex_show": "plex_show_id", + "playlist": "playlist_id", +} +PIN_KINDS = tuple(PIN_COLUMNS) + + +def resolve_ref(db: Session, user: User, kind: str, ref: str) -> tuple[str, object] | None: + """Translate the client's (kind, ref) into the (column, value) the row stores, or None. + + The client speaks the ids the rest of the API speaks: a video id, a channel id, a Plex + `rating_key`, a playlist id. Plex's internal integer keys never leave the backend, so the + translation belongs here rather than in five call sites. + + The COLUMN half of every answer comes from `PIN_COLUMNS`; only the LOOKUP differs per kind + (a direct `db.get`, a rating-key translation, an int parse plus an ownership check). Before the + E24 review the branches spelled the column names out again and `PIN_COLUMNS` had no readers at + all — a constant that called itself the one vocabulary while nothing spoke it.""" + col = PIN_COLUMNS.get(kind) + if col is None: + return None + if kind == "video": + return (col, ref) if db.get(Video, ref) is not None else None + if kind == "channel": + return (col, ref) if db.get(Channel, ref) is not None else None + if kind == "plex_item": + it = by_rating_key(db, PlexItem, ref) + return (col, it.id) if it is not None else None + if kind == "plex_show": + sh = by_rating_key(db, PlexShow, ref) + return (col, sh.id) if sh is not None else None + try: + pid = int(ref) + except ValueError: + return None + pl = db.get(Playlist, pid) + # A playlist is per-user, unlike the other four (which are shared catalog rows whose PRIVACY + # lives in the state tables). Someone else's list must read as "no such thing", not as a 403 — + # the same reason `owned_or_404` 404s rather than 403s. + return (col, pl.id) if pl is not None and pl.user_id == user.id else None + + +def list_pins(db: Session, user: User) -> list[ComfortPin]: + return list( + db.execute( + select(ComfortPin) + .where(ComfortPin.user_id == user.id) + .order_by(ComfortPin.position.asc(), ComfortPin.id.asc()) + ) + .scalars() + .all() + ) + + +def next_position(db: Session, user: User) -> int: + """New pins land at the end of the shelf. + + `max(position) + 1`, not the row count. Positions are never renumbered on removal — `remove_pin` + deletes the row, and every FK cascade silently removes a pin when the thing it points at goes + away — so past the first hole a count under-counts and would drop the new pin into the MIDDLE of + the shelf, or onto a position another pin already holds (nothing in the schema forbids that). + Same idiom as `routes/playlists._next_playlist_position`.""" + return ( + db.scalar( + select(func.coalesce(func.max(ComfortPin.position), -1)).where( + ComfortPin.user_id == user.id + ) + ) + + 1 + ) + + +def _shelf_card(db: Session, user: User, pin: ComfortPin) -> dict | None: + """One pin as a unified card with its resolved `next`. None when the pinned row has gone (the FK + cascade removes the pin itself, so this is only the window between a delete and a refetch).""" + card: dict | None = None + nxt: dict | None = None + + if pin.video_id is not None: + row = queries.yt_video_row(db, user, pin.video_id) + if row is None: + return None + card = cards.from_video(row) + nxt = dict(card) # a video plays itself + + elif pin.channel_id is not None: + ch = db.get(Channel, pin.channel_id) + if ch is None: + return None + card = cards.from_channel(ch, queries.channel_unwatched_count(db, user, ch.id)) + row = queries.channel_next_video(db, user, ch.id) + nxt = cards.from_video(row) if row else None + + elif pin.plex_item_id is not None: + it = db.get(PlexItem, pin.plex_item_id) + if it is None: + return None + st = rollup.item_state_map(db, user.id, [it.id]).get(it.id) + card = cards.from_plex(leaf_card(it, st)) + nxt = dict(card) # a movie or a single episode plays itself + + elif pin.plex_show_id is not None: + sh = db.get(PlexShow, pin.plex_show_id) + if sh is None: + return None + status = rollup.state_map(db, user.id, [sh.id]).get(sh.id, "new") + card = cards.from_plex(show_card(sh, status)) + ep = queries.next_episode(db, user.id, sh.id) + nxt = cards.from_plex(ep) if ep else None + + elif pin.playlist_id is not None: + pl = db.get(Playlist, pin.playlist_id) + if pl is None: + return None + count, row = queries.playlist_summary(db, user, pl.id) + card = cards.from_playlist(pl, count, row.get("thumbnail_url") if row else None) + nxt = cards.from_video(row) if row else None + + if card is None: + return None + # A COPY, never the card itself: a self-playing pin (a video, a movie) would otherwise make + # `card["next"] is card`, which serializes as a circular reference and 500s the whole shelf. The + # copy is taken before `pin_id`/`next` are attached, so `next` stays a plain playable card. + card["pin_id"] = pin.id + card["next"] = nxt + return card + + +def shelf(db: Session, user: User) -> list[dict]: + """The whole shelf, in the user's own order.""" + out = [] + for pin in list_pins(db, user): + card = _shelf_card(db, user, pin) + if card is not None: + out.append(card) + return out + + +def playables(shelf_cards: list[dict]) -> list[dict]: + """The `next` of every shelf entry that has one — what the hero may draw from. A pinned channel + with nothing unwatched left, or an empty playlist, simply is not an answer right now.""" + return [c["next"] for c in shelf_cards if c.get("next")] diff --git a/backend/app/discover/queries.py b/backend/app/discover/queries.py new file mode 100644 index 00000000..403bd8c9 --- /dev/null +++ b/backend/app/discover/queries.py @@ -0,0 +1,348 @@ +"""The row fetchers behind the Home surface — one place where "what is startable right now" is +expressed as SQL, for both sources. + +Two rules shape this module: + +**Reuse the query layer, not the route.** The YouTube half calls `feedquery.filtered_query` / +`sort_keys`, so the Home surface inherits the feed's own definitions of subscribed, blocked, +explore-visible and in-progress for free, and cannot drift from them. The Plex half calls +`plex.rollup` and the `plex.cards` serializers rather than `routes/plex/discover.py`: that route +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()`. +""" + +from sqlalchemy import and_, func, or_, select +from sqlalchemy.orm import Session, aliased + +from app.discover.budget import band +from app.feedquery import ( + feed_columns, + filtered_query, + not_blocked_cond, + order_by, + serialize, + sort_keys, +) +from app.models import ( + Channel, + PlaylistItem, + PlexItem, + PlexShow, + PlexState, + User, + Video, + VideoState, +) +from app.plex import rollup +from app.plex.cards import leaf_card, movie_card, show_card +from app.plex.query import apply_show_state_filter, content_libs + +# How many rows a single rail may carry. Matches `routes/plex/discover.py`'s RAIL_LIMIT on purpose — +# the two surfaces should feel like the same app, and a rail is a rail. +RAIL_LIMIT = 20 + +# The feed parameters a Home rail runs with. Spelled out rather than borrowed from the route's +# `_feed_params` defaults, because those are an HTTP contract that may change for the feed's own +# reasons; the landing page's scope should not move with them silently. +_FEED_BASE: dict = { + "tags": [], + "tag_modes": {}, + "untagged": False, + "channel_ids": [], + "q": None, + "min_duration": None, + "max_duration": None, + "max_age_days": None, + "published_after": None, + "published_before": None, + "show_normal": True, + "include_shorts": False, + "include_live": False, + "show": "unwatched", + "scope": "my", + "library_source": "organic", +} + + +def _seeded(col, seed: int): + """The feed's deterministic shuffle expression, reused for Plex so one seed moves both halves.""" + return func.md5(func.concat(col, str(seed))) + + +# ── YouTube ─────────────────────────────────────────────────────────────────────────────────────── + + +def yt_in_progress(db: Session, user: User, limit: int = RAIL_LIMIT) -> list[tuple[object, dict]]: + """Started-but-not-finished videos, most recently progressed first, as (progressed_at, row). + + The predicate is `feedquery`'s `show="in_progress"` verbatim (a stored resume position, status not + watched/hidden) — but the query is built here rather than through `filtered_query` because this + rail wants an ordering by `progress_updated_at`, and that column lives on the state alias the feed + query keeps private. Driving from `video_states` is also the faster plan: it is a per-user table of + a few hundred rows, against a catalog of many thousands. + + Building it here means the feed's OTHER predicates have to be chosen deliberately rather than + inherited. `not_blocked_cond` is shared, because "never show me this channel" outranks "you + started it" — blocking leaves the state row untouched, so without this the video the user just + banned heads the resume rail. The feed's SCOPE clauses (subscribed / via_explore accessibility) + are deliberately NOT applied, for the reason `yt_video_row` gives: something you have already + started must stay resumable even after you unsubscribe or an exploration expires.""" + st = aliased(VideoState) + q = ( + select(*feed_columns(user, st), st.progress_updated_at.label("progressed_at")) + .join(Channel, Channel.id == Video.channel_id) + .join(st, and_(st.video_id == Video.id, st.user_id == user.id)) + .where( + st.position_seconds > 0, + st.status.notin_(("watched", "hidden")), + Video.unavailable_since.is_(None), + not_blocked_cond(user), + ) + .order_by(st.progress_updated_at.desc().nullslast()) + .limit(limit) + ) + return [(r.progressed_at, serialize(r)) for r in db.execute(q).all()] + + +def yt_surprise(db: Session, user: User, budget: str, seed: int, limit: int) -> list[dict]: + """Unwatched videos from the user's own feed scope, in seeded-shuffle order, inside the budget. + + 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. + 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] + + +def channel_next_video(db: Session, user: User, channel_id: str) -> dict | None: + """The newest unwatched video of a pinned channel — what "put on that channel" resolves to.""" + # `library_source="all"`: a pinned channel means the whole channel, including videos that only + # ever arrived through a search. The feed's "organic" default is a feed-scoping choice, not a + # statement about what belongs to a channel. + params = dict(_FEED_BASE, channel_ids=[channel_id], scope="all", library_source="all") + query, rank_expr = filtered_query(db, user, **params) + keys = sort_keys("newest", 0, rank_expr) + row = db.execute(query.order_by(*order_by(keys)).limit(1)).first() + return serialize(row) if row is not None else None + + +def yt_video_row(db: Session, user: User, video_id: str) -> dict | None: + """One video as a feed row, with this user's watch state. + + Built directly rather than through `filtered_query` on purpose: a PINNED video must keep working + even when it falls outside the feed's scope — the user unsubscribed from the channel, or blocked + it, or the video only ever arrived through a search. The shelf is the surface you fall back to; + it silently losing an entry because a filter moved is exactly the failure it must not have.""" + st = aliased(VideoState) + row = db.execute( + select(*feed_columns(user, st)) + .join(Channel, Channel.id == Video.channel_id) + .outerjoin(st, and_(st.video_id == Video.id, st.user_id == user.id)) + .where(Video.id == video_id) + ).first() + return serialize(row) if row is not None else None + + +def channel_unwatched_count(db: Session, user: User, channel_id: str) -> int: + """How many unwatched videos a pinned channel is holding — the tile's only honest promise, since a + channel has no runtime of its own.""" + params = dict(_FEED_BASE, channel_ids=[channel_id], scope="all", library_source="all") + query, _ = filtered_query(db, user, **params) + return int(db.execute(select(func.count()).select_from(query.subquery())).scalar() or 0) + + +def playlist_summary(db: Session, user: User, playlist_id: int) -> tuple[int, dict | None]: + """(item count, the row to play next) for a pinned playlist — its first not-yet-watched item, or + its first item once the whole list is watched. The same row doubles as the tile's artwork, so a + shelf of playlists is scannable instead of a row of blank boxes.""" + st = aliased(VideoState) + rows = db.execute( + select(*feed_columns(user, st), PlaylistItem.position) + .join(PlaylistItem, PlaylistItem.video_id == Video.id) + .join(Channel, Channel.id == Video.channel_id) + .outerjoin(st, and_(st.video_id == Video.id, st.user_id == user.id)) + .where(PlaylistItem.playlist_id == playlist_id) + .order_by(PlaylistItem.position.asc()) + ).all() + if not rows: + return 0, None + nxt = next((r for r in rows if (r.status or "new") not in ("watched", "hidden")), rows[0]) + return len(rows), serialize(nxt) + + +# ── Plex ────────────────────────────────────────────────────────────────────────────────────────── + + +def _unseen_movie_conds(uid: int): + """A movie the user has neither finished nor hidden. A LEFT-JOIN-IS-NULL would also drop rows that + merely carry a resume position, which are exactly the ones Continue watching wants back.""" + seen = ( + select(1) + .select_from(PlexState) + .where( + PlexState.item_id == PlexItem.id, + PlexState.user_id == uid, + or_( + PlexState.status.in_(("watched", "hidden")), + # Started counts as "not a surprise" too — see the note in `yt_surprise`. + PlexState.position_seconds > 0, + ), + ) + .exists() + ) + return ~seen + + +def plex_in_progress(db: Session, uid: int, limit: int = RAIL_LIMIT) -> list[tuple[object, dict]]: + """In-progress movies + partially-watched shows as (progressed_at, card). + + Mirrors `routes/plex/discover.py::_continue_watching` — same `PlexState` predicate for movies (a + NULL status still counts as started, so `notin_` alone would drop it), and the same shared + `rollup.agg_subquery` + `apply_show_state_filter` for shows, so the two rails cannot disagree on + what "in progress" means. What it does NOT inherit is that route's `LibraryFilters` narrowing: + this surface has no Plex sidebar to read a filter from.""" + movie_libs, show_libs = content_libs(db) + out: list[tuple[object, dict]] = [] + + if movie_libs: + mq = ( + select(PlexItem, PlexState) + .join(PlexState, and_(PlexState.item_id == PlexItem.id, PlexState.user_id == uid)) + .where( + PlexItem.kind == "movie", + PlexItem.library_id.in_(movie_libs), + PlexState.position_seconds > 0, + or_( + PlexState.status.is_(None), + PlexState.status.notin_(("watched", "hidden")), + ), + ) + .order_by(PlexState.progress_updated_at.desc().nullslast()) + .limit(limit) + ) + for it, st in db.execute(mq).all(): + out.append((st.progress_updated_at, movie_card(it, st))) + + if show_libs: + prog = rollup.agg_subquery(db, uid, include_resumed=True).subquery() + sq = apply_show_state_filter( + select(PlexShow, prog.c.resumed_at).join(prog, prog.c.show_id == PlexShow.id), + prog.c.total, + prog.c.watched, + prog.c.inprog, + "in_progress", + ).where(PlexShow.library_id.in_(show_libs)) + for sh, resumed_at in db.execute( + sq.order_by(prog.c.resumed_at.desc().nullslast()).limit(limit) + ).all(): + out.append((resumed_at, show_card(sh, "in_progress"))) + + return out + + +def plex_surprise(db: Session, uid: int, budget: str, seed: int, limit: int) -> list[dict]: + """Unseen playable movies inside the budget, in seeded-shuffle order. + + Movies only. A show is not a thing you can start playing, and resolving every candidate show to + its next episode just to draw three cards would be a query per row for a rail whose whole value is + that it answers instantly — shows reach this surface through Continue watching and the comfort + shelf, which is where "put the next episode on" actually belongs. + + No `playable` filter, matching the Plex surprise rail: `PlexItem.playable` is a playback CLASS + ("direct" / "remux" / "transcode"), not a yes/no, and NULL there means "not probed yet" rather than + "will not play". Excluding un-probed items would quietly hide a fresh library from the one surface + that is supposed to have an answer. The class travels on the card, so the client still shows it.""" + movie_libs, _ = content_libs(db) + 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) + ) + 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() + states = rollup.item_state_map(db, uid, [it.id for it in items]) + return [movie_card(it, states.get(it.id)) for it in items] + + +# Watch states that take an episode out of the running for "what would play next". +_DONE = ("watched", "hidden") + + +def next_episode(db: Session, uid: int, show_id: int) -> dict | None: + """What "put that show on" means: the episode in progress, else the first unwatched one, else the + first — over the same `(season, episode)` broadcast ordering `detail.py::_episode_nav` uses. + + Three ordered `LIMIT 1` probes rather than one scan picked apart in Python. The Python version + loaded EVERY episode of the show to choose one of them — 236 rows for Friends, 227 for JAG — which + the comfort shelf could just about afford (a handful of hand-picked pins) and the resume rail + could not, since it resolves a show per rail row on the landing page. + + Returned as a `leaf_card` so it carries the show title the episode row itself does not.""" + asc = ( + PlexItem.season_number.asc().nullsfirst(), + PlexItem.episode_number.asc().nullsfirst(), + ) + base = ( + select(PlexItem, PlexState) + .outerjoin(PlexState, and_(PlexState.item_id == PlexItem.id, PlexState.user_id == uid)) + .where(PlexItem.show_id == show_id, PlexItem.kind == "episode") + .limit(1) + ) + # In progress — the LAST one in broadcast order, which is `plex.rollup.rollup`'s `inprog[-1]`, + # i.e. what the show page's own Resume button plays. Two episodes of one show are in progress at + # once more often than it sounds (`watch_sync._reconcile_inprogress_movies` documents that a + # scrub to the end leaves a stale `status='new', position>0` row until the daily reconcile), and + # taking opposite ends of that set is how one show came to have two different "continue" answers: + # the Home tile ranked it by its LATEST touch and then offered the EARLIEST started episode. + # `position_seconds > 0` already requires the state row to exist, so no null guard. + pick = db.execute( + base.where(PlexState.position_seconds > 0, PlexState.status.notin_(_DONE)).order_by( + PlexItem.season_number.desc().nullslast(), + PlexItem.episode_number.desc().nullslast(), + ) + ).first() + # Unwatched — INCLUDING an episode with no state row at all, which is the common case and the one + # a bare `status NOT IN (...)` silently drops (NULL is not false, it is unknown, so the row fails + # the filter). + pick = ( + pick + or db.execute( + base.where(or_(PlexState.id.is_(None), PlexState.status.notin_(_DONE))).order_by(*asc) + ).first() + ) + # Fully watched show: start it again from the top. + pick = pick or db.execute(base.order_by(*asc)).first() + if pick is None: + return None + + ep, st = pick + show = db.get(PlexShow, show_id) + return leaf_card( + ep, st, show_title=show.title if show else None, show_rk=show.rating_key if show else None + ) diff --git a/backend/app/discover/rails.py b/backend/app/discover/rails.py new file mode 100644 index 00000000..0598e0a1 --- /dev/null +++ b/backend/app/discover/rails.py @@ -0,0 +1,97 @@ +"""The Home surface's rails: cross-source lists, assembled from `queries.py` and emitted as unified +cards. + +The whole point of this module is that a rail is ONE list, not a YouTube list beside a Plex list. Two +side-by-side rails would just move the choice the user could not make ("which half do I look at?") one +level up, which is the opposite of what this epic is for. +""" + +from sqlalchemy.orm import Session + +from app import sysconfig +from app.discover import cards, queries +from app.discover.queries import RAIL_LIMIT +from app.models import PlexShow, User +from app.plex.lookups import by_rating_key +from app.plex.rollup import NO_TS + + +def plex_available(db: Session) -> bool: + """Plex is a deployment-level toggle, not a per-user one (see `routes/me.py`). With it off, the + Home rails are YouTube-only and say nothing about Plex.""" + return sysconfig.get_bool(db, "plex_enabled") + + +def playable(db: Session, uid: int, card: dict) -> dict | None: + """The thing that would actually START from this card — itself for anything with a runtime, and + for a Plex SHOW the episode that is next (in progress, else the first unwatched, else the first). + + ONE function, because three surfaces need the same answer and a second implementation is how the + shelf and the rail came to disagree in the first place: the hero calls it to decide what it is + offering, `continue_watching` calls it to fill each show tile's `next`, and the comfort shelf + resolves the same way in `pins`. None when the show has no episodes mirrored yet. + + A card that already carries a resolved `next` (a rail row) is taken at its word rather than + re-queried — the hero reads the rail it was handed.""" + if not (card.get("source") == "plex" and card.get("kind") == "show"): + return card + if "next" in card: + return card["next"] + sh = by_rating_key(db, PlexShow, card["id"]) + if sh is None: + return None + ep = queries.next_episode(db, uid, sh.id) + return cards.from_plex(ep) if ep else None + + +def continue_watching(db: Session, user: User, limit: int = RAIL_LIMIT) -> list[dict]: + """One resume rail across both sources, most recently progressed first. + + A Plex SHOW enters as a show CARD — that is what the Plex rail shows, and it is the artwork the + user recognises — but it carries `next`: the episode that would actually start. Without it a + click on a half-watched series landed on the show page with six seasons to choose between, which + is the decision this whole surface exists to remove (the comfort shelf and the hero already + resolved; only this rail did not). The client plays `next ?? card`, so the tile stays a poster and + the click stays one click.""" + rows: list[tuple[object, dict]] = [ + (ts, cards.from_video(r)) for ts, r in queries.yt_in_progress(db, user, limit) + ] + if plex_available(db): + rows += [(ts, cards.from_plex(c)) for ts, c in queries.plex_in_progress(db, user.id, limit)] + rows.sort(key=lambda r: r[0] or NO_TS, reverse=True) + out = [card for _, card in rows[:limit]] + # Resolve AFTER the slice: only the rows that survive it are worth a query. + for card in out: + if card["kind"] == "show": + card["next"] = playable(db, user.id, card) + return out + + +def surprise(db: Session, user: User, budget: str, seed: int, limit: int = 3) -> list[dict]: + """The hero's draw pool: unseen things that fit the budget, from both sources under ONE seed. + + Interleaved rather than concatenated — the same alternation `routes/plex/discover.py` uses for its + movie/show split — so a short pool never comes back all-YouTube or all-Plex just because one side + happened to be queried first. + + WHICH source leads is decided by the seed, not by a hardcoded order. The hero draws a pool of ONE, + so a fixed order would mean the Plex library could never answer "egy este" while a single YouTube + video also fit — the reroll would look broken and the cross-source promise would quietly be a + YouTube promise. Parity on the same seed that orders the rows keeps it deterministic: the same + seed always gives the same answer, and pressing "Mást kérek" moves both halves together.""" + halves = [ + (cards.from_video, queries.yt_surprise(db, user, budget, seed, limit)), + ( + cards.from_plex, + queries.plex_surprise(db, user.id, budget, seed, limit) if plex_available(db) else [], + ), + ] + if seed % 2: + halves.reverse() + + out: list[dict] = [] + for i in range(max(len(rows) for _, rows in halves)): + for to_card, rows in halves: + if i < len(rows): + out.append(to_card(rows[i])) + return out[:limit] diff --git a/backend/app/feedquery.py b/backend/app/feedquery.py index 80e72d3b..35cd8cfd 100644 --- a/backend/app/feedquery.py +++ b/backend/app/feedquery.py @@ -117,6 +117,19 @@ def tags_by_category(db: Session, tag_ids: list[int]) -> dict[str, list[int]]: return out +def not_blocked_cond(user: User): + """Per-user channel blocklist: never show this user videos from a channel they blocked + (applies to feed, Library, explore, the channel page and the Home resume rail alike). + + A shared expression rather than a clause each surface writes for itself. Blocking does not touch + `video_states`, so a half-watched video from a newly blocked channel survives with its resume + position — and the one query that copied only PART of this predicate set is exactly how that + video came back as the landing page's suggestion after the user had said "never again".""" + return Video.channel_id.notin_( + select(BlockedChannel.channel_id).where(BlockedChannel.user_id == user.id) + ) + + def filtered_query( db: Session, user: User, @@ -207,13 +220,7 @@ def filtered_query( ) ) - # Per-user channel blocklist: never show this user videos from a channel they blocked - # (applies to feed, Library, explore, and the channel page alike). - query = query.where( - Video.channel_id.notin_( - select(BlockedChannel.channel_id).where(BlockedChannel.user_id == user.id) - ) - ) + query = query.where(not_blocked_cond(user)) # Source filter. In the shared Library ("all" scope) it uses the GLOBAL via_search flag # (search-discovered by anyone). In "my" scope it's the user's OWN provenance: diff --git a/backend/app/main.py b/backend/app/main.py index 1cfeccf8..1386116c 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -33,6 +33,7 @@ from app.routes import ( auth as auth_routes, channels, config as config_routes, + discover as discover_routes, downloads, feed, health, @@ -187,6 +188,7 @@ app.include_router(auth_routes.router) app.include_router(sync.router) app.include_router(tags.router) app.include_router(feed.router) +app.include_router(discover_routes.router) app.include_router(search_routes.router) app.include_router(me.router) app.include_router(notifications.router) diff --git a/backend/app/models.py b/backend/app/models.py index 8d16706f..02a2895a 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -3,6 +3,7 @@ from datetime import date, datetime from sqlalchemy import ( BigInteger, Boolean, + CheckConstraint, Computed, Date, DateTime, @@ -1406,3 +1407,52 @@ class PlexPlaylistItem(Base): ForeignKey("plex_items.id", ondelete="CASCADE"), index=True ) position: Mapped[int] = mapped_column(Integer, default=0, server_default="0", nullable=False) + + +class ComfortPin(Base, TimestampMixin): + """One entry on a user's comfort shelf — the anti-paralysis fallback: "just put on X" (E24). + + Deliberately NOT a boolean on `VideoState`/`PlexState`, and NOT a polymorphic `(kind, ref)` string + pair. A flag cannot ORDER a shelf and cannot hold a channel or a show — and comfort media is + usually "put on X", a series or a channel, not one particular video. A string ref would order and + hold anything, but it would also lose the foreign key: a Plex item dropped from the library, or an + unsubscribed channel, would leave a dead tile on the one surface whose whole job is to be + reliable. So instead: one nullable CASCADE-ing FK per pinnable kind, and a check constraint that + exactly one of them is set. + + That constraint is the house rule applied to a five-way union — the invariant lives in the schema, + not in a comment every insert site has to remember. `kind`/`ref` below derive FROM the columns, so + there is no second vocabulary to keep in sync. + + NULLs are DISTINCT in a Postgres unique index, so the five per-kind unique constraints each stop a + double-pin of the same thing without colliding across kinds.""" + + __tablename__ = "comfort_pins" + __table_args__ = ( + CheckConstraint( + "num_nonnulls(video_id, channel_id, plex_item_id, plex_show_id, playlist_id) = 1", + name="ck_comfort_pin_one_ref", + ), + UniqueConstraint("user_id", "video_id", name="uq_comfort_pin_video"), + UniqueConstraint("user_id", "channel_id", name="uq_comfort_pin_channel"), + UniqueConstraint("user_id", "plex_item_id", name="uq_comfort_pin_plex_item"), + UniqueConstraint("user_id", "plex_show_id", name="uq_comfort_pin_plex_show"), + UniqueConstraint("user_id", "playlist_id", name="uq_comfort_pin_playlist"), + ) + + id: Mapped[int] = mapped_column(primary_key=True) + user_id: Mapped[int] = mapped_column(ForeignKey("users.id", ondelete="CASCADE"), index=True) + + video_id: Mapped[str | None] = mapped_column(ForeignKey("videos.id", ondelete="CASCADE")) + channel_id: Mapped[str | None] = mapped_column(ForeignKey("channels.id", ondelete="CASCADE")) + plex_item_id: Mapped[int | None] = mapped_column( + ForeignKey("plex_items.id", ondelete="CASCADE") + ) + plex_show_id: Mapped[int | None] = mapped_column( + ForeignKey("plex_shows.id", ondelete="CASCADE") + ) + playlist_id: Mapped[int | None] = mapped_column(ForeignKey("playlists.id", ondelete="CASCADE")) + + # 0-based shelf order. The shelf is short and hand-curated, so a dense integer the reorder + # endpoint rewrites wholesale beats a fractional index nobody would ever exercise here. + position: Mapped[int] = mapped_column(Integer, default=0, server_default="0", nullable=False) diff --git a/backend/app/plex/query.py b/backend/app/plex/query.py index 19f21885..c3fd6f7e 100644 --- a/backend/app/plex/query.py +++ b/backend/app/plex/query.py @@ -8,8 +8,9 @@ from datetime import datetime, timedelta, timezone from sqlalchemy import and_, case, func, or_, select, type_coerce from sqlalchemy.dialects.postgresql import JSONB +from sqlalchemy.orm import Session -from app.models import PlexItem +from app.models import PlexItem, PlexLibrary TS_CONFIG = "public.unaccent_simple" @@ -342,3 +343,26 @@ def apply_show_state_filter(query, tot, wat, inp, show: str): if show == "unwatched": return query.where(wat == 0, inp == 0) return query + + +def content_libs(db: Session) -> tuple[list[int], list[int]]: + """The BROWSABLE content libraries, partitioned into (movie_lib_ids, show_lib_ids) — the single + source of the browse scope. Excludes HU overlay libraries (`overlay_of` set): those are never + browsed on their own, they only lend their Hungarian metadata to the EN twin's rows (E5 S5). + + Lives here, in the query layer, rather than in `routes/plex/_shared.py` where it started: E24's + Home surface needs the same scope, and a domain package importing from `routes/` would invert the + one-way dependency CLAUDE.md draws. It never touched request state, so nothing was lost by the + move.""" + libs = ( + db.execute( + select(PlexLibrary).where( + PlexLibrary.enabled.is_(True), PlexLibrary.overlay_of.is_(None) + ) + ) + .scalars() + .all() + ) + movie_lib_ids = [lb.id for lb in libs if lb.kind == "movie"] + show_lib_ids = [lb.id for lb in libs if lb.kind == "show"] + return movie_lib_ids, show_lib_ids diff --git a/backend/app/plex/rollup.py b/backend/app/plex/rollup.py index 4ea9d228..53211250 100644 --- a/backend/app/plex/rollup.py +++ b/backend/app/plex/rollup.py @@ -3,11 +3,25 @@ Drives the show/season grid badges and the Resume / Play-from-start / mark-all buttons. """ +from datetime import datetime, timezone + from sqlalchemy import and_, case, func, or_, select from sqlalchemy.orm import Session, aliased from app.models import PlexItem, PlexState +# Sort sentinel for a rail row whose timestamp is missing, so `key=lambda r: r[0] or NO_TS` reads the +# same at every rail that merges two type-homogeneous lists in Python. +# +# It is NOT a crash fix, whatever the E24 commit that introduced it says. The `(ts is not None, ts)` +# key it replaced cannot raise: tuple comparison finds the first UNEQUAL element via `==`, so two +# `(False, None)` keys compare equal and `None < None` is never reached — verified by execution +# during the E24 epic-end review, over every permutation of null/non-null rows. What the sentinel +# buys is one idiom instead of two, and a key that says "missing sorts last" outright rather than +# encoding it in a boolean flag. A BARE `r[0]` would genuinely raise, which is the mutation the +# rails' tests do guard against. +NO_TS = datetime.min.replace(tzinfo=timezone.utc) + def show_status(total: int, watched: int, inprog: int) -> str: """Roll a show's episodes up into a single watch state: fully watched, partially started, or new.""" diff --git a/backend/app/routes/discover.py b/backend/app/routes/discover.py new file mode 100644 index 00000000..f029f0e2 --- /dev/null +++ b/backend/app/routes/discover.py @@ -0,0 +1,197 @@ +"""E24 · the Home surface's HTTP shell. + +Thin by construction: every decision — what counts as in progress, what fits a time budget, which +tier the hero comes from — lives in `app/discover/`, so the scheduler, a future CLI or a test can ask +the same questions without a request. This file validates, gates and serializes. + +The consolidated `GET /api/discover` mirrors the Plex Discover route's one-payload shape (E5 S4): a +landing page that fires four requests to answer one question would spend its whole budget on the +thing it exists to remove. +""" + +from typing import Literal + +from fastapi import APIRouter, Depends, HTTPException +from pydantic import BaseModel +from sqlalchemy import select, update +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + +from app.auth import current_user, require_human +from app.db import get_db +from app.discover import hero as hero_mod +from app.discover import pins, rails +from app.discover.budget import Budget, DEFAULT_BUDGET +from app.models import ComfortPin, User +from app.routes._common import owned_or_404 + +router = APIRouter(prefix="/api/discover", tags=["discover"]) + + +class PinIn(BaseModel): + # From `pins.PIN_KINDS`, not a second hand-written list: the vocabulary, the schema columns and + # this request contract have to move together, and re-typing them here is how they would not. + kind: Literal[*pins.PIN_KINDS] + # The id the rest of the API speaks for that kind: a video id, a channel id, a Plex rating_key, + # or a playlist id. `pins.resolve_ref` does the translation to the row's internal key. + ref: str + + +class ReorderIn(BaseModel): + ids: list[int] + + +@router.get("") +def get_discover( + budget: Budget = DEFAULT_BUDGET, + seed: int = 0, + user: User = Depends(current_user), + db: Session = Depends(get_db), +) -> dict: + """Everything the Home page renders, in one pass. + + `resume` and `shelf` are built once and handed to the hero, so the two rails the hero draws from + are not queried twice for a single page load.""" + resume = rails.continue_watching(db, user) + shelf = pins.shelf(db, user) + return { + "budget": budget, + "hero": hero_mod.pick(db, user, budget, seed, resume=resume, shelf=shelf), + "continue_watching": resume, + "comfort": shelf, + } + + +# How many already-shown cards a reroll may carry, matched to the client's own rolling window +# (`SEEN_WINDOW` in `Home.tsx`). A cheap sanity clamp, and deliberately no more than that: `exclude` +# never reaches SQL — `hero.pick` uses it only as set membership over an already-bounded candidate +# pool (the resume rail's `RAIL_LIMIT` rows, the shelf, `SURPRISE_DRAW` surprise draws) — so there is +# no scan for a crafted URL to unbound, and the set is built in full before this slice anyway. The +# comment that claimed otherwise was corrected in round 2; the request line's own length is what +# actually bounds a hostile `exclude`. +MAX_EXCLUDE = 32 + + +@router.get("/pick") +def get_pick( + budget: Budget = DEFAULT_BUDGET, + seed: int = 0, + exclude: str = "", + start: int = 0, + user: User = Depends(current_user), + db: Session = Depends(get_db), +) -> dict: + """The "Mást kérek" reroll: a hero on its own, for a new seed and/or a new budget. + + One seed drives BOTH sources (see `discover/queries.py`) — otherwise a reroll would re-draw the + Plex half while the YouTube half stood still, and the button would look broken half the time. + + `exclude` is a comma-separated list of `:` keys the user has already been shown. It + is what makes the button honest: a seed alone only moves the surprise tier, so a user with + something half-finished got the same card back every press. + + `start` is the press count: it rotates which tier answers first, so consecutive presses offer a + different KIND of answer instead of walking down the resume rail. Taken modulo the tier count in + `hero.pick`, so any integer is valid and no value can 422 a button.""" + skip = {p for p in exclude.split(",") if p} + return { + "budget": budget, + "hero": hero_mod.pick( + db, user, budget, seed, exclude=set(list(skip)[:MAX_EXCLUDE]), start=start + ), + } + + +@router.get("/pins") +def list_pins( + user: User = Depends(current_user), + db: Session = Depends(get_db), +) -> dict: + """The shelf on its own — for the shelf editor, which needs it without the rest of the page.""" + return {"items": pins.shelf(db, user)} + + +@router.post("/pins") +def add_pin( + body: PinIn, + user: User = Depends(require_human), + db: Session = Depends(get_db), +) -> dict: + """Pin something to the comfort shelf. + + `require_human`, like every other personal-state write: the shared demo account must not be able + to curate a communal shelf. Re-pinning something already on the shelf is a no-op that returns the + existing pin rather than a 409 — the gesture is "make sure this is on my shelf", and the unique + constraint is the mechanism, not the message.""" + resolved = pins.resolve_ref(db, user, body.kind, body.ref) + if resolved is None: + raise HTTPException(status_code=404, detail="Unknown item") + column, value = resolved + + existing = db.execute( + select(ComfortPin).where( + ComfortPin.user_id == user.id, + getattr(ComfortPin, column) == value, + ) + ).scalar_one_or_none() + if existing is not None: + return {"ok": True, "id": existing.id, "created": False} + + pin = ComfortPin(user_id=user.id, position=pins.next_position(db, user), **{column: value}) + db.add(pin) + try: + db.commit() + except IntegrityError: + # A concurrent pin of the same thing from a second tab. The unique constraint did its job; + # the user's intent ("be on my shelf") is satisfied either way, so report the winner rather + # than a 500 the user can do nothing with. + db.rollback() + winner = db.execute( + select(ComfortPin).where( + ComfortPin.user_id == user.id, + getattr(ComfortPin, column) == value, + ) + ).scalar_one_or_none() + if winner is None: + # Not the unique constraint, then: the only other way in is the target row being pruned + # between `resolve_ref` confirming it and this INSERT (a Plex sync dropping a removed + # item), which trips the FK. Semantically that is the same "no such thing" the pre-check + # returns above — re-raising handed the user a 500 they could do nothing with. + raise HTTPException(status_code=404, detail="Unknown item") + return {"ok": True, "id": winner.id, "created": False} + return {"ok": True, "id": pin.id, "created": True} + + +@router.delete("/pins/{pin_id}") +def remove_pin( + pin_id: int, + user: User = Depends(require_human), + db: Session = Depends(get_db), +) -> dict: + pin = owned_or_404(db, ComfortPin, pin_id, user) + db.delete(pin) + db.commit() + return {"deleted": pin_id} + + +@router.post("/pins/reorder") +def reorder_pins( + body: ReorderIn, + user: User = Depends(require_human), + db: Session = Depends(get_db), +) -> dict: + """Rewrite the shelf order from a full list of pin ids. + + Whole-list rather than a move-one delta: the shelf is short, and a full rewrite cannot leave two + tiles claiming the same slot the way an interrupted sequence of single moves can. Ids that are not + this user's are ignored rather than 404'd — a stale tab replaying an order that includes a pin + deleted meanwhile should reorder what still exists, not fail wholesale.""" + owned = {p.id for p in pins.list_pins(db, user)} + for position, pin_id in enumerate(i for i in body.ids if i in owned): + db.execute( + update(ComfortPin) + .where(ComfortPin.id == pin_id, ComfortPin.user_id == user.id) + .values(position=position) + ) + db.commit() + return {"ok": True} diff --git a/backend/app/routes/plex/_shared.py b/backend/app/routes/plex/_shared.py index 0e50040b..2a193eab 100644 --- a/backend/app/routes/plex/_shared.py +++ b/backend/app/routes/plex/_shared.py @@ -3,13 +3,9 @@ import logging from fastapi import HTTPException -from sqlalchemy import select from sqlalchemy.orm import Session -from app.models import ( - PlexItem, - PlexLibrary, -) +from app.models import PlexItem from app.plex.lookups import by_rating_key log = logging.getLogger("siftlode.plex") @@ -25,21 +21,3 @@ def item_or_404(db: Session, rating_key: str) -> PlexItem: def _norm_lang(lang: str | None) -> str: """The overlay has an EN base + an optional HU twin; anything other than `hu` serves EN.""" return "hu" if lang == "hu" else "en" - - -def _content_libs(db: Session) -> tuple[list[int], list[int]]: - """The BROWSABLE content libraries, partitioned into (movie_lib_ids, show_lib_ids) — the single - source of the browse scope. Excludes HU overlay libraries (`overlay_of` set): those are never - browsed on their own, they only lend their Hungarian metadata to the EN twin's rows (E5 S5).""" - libs = ( - db.execute( - select(PlexLibrary).where( - PlexLibrary.enabled.is_(True), PlexLibrary.overlay_of.is_(None) - ) - ) - .scalars() - .all() - ) - movie_lib_ids = [lb.id for lb in libs if lb.kind == "movie"] - show_lib_ids = [lb.id for lb in libs if lb.kind == "show"] - return movie_lib_ids, show_lib_ids diff --git a/backend/app/routes/plex/browse.py b/backend/app/routes/plex/browse.py index 0ff9dff8..5cdda59a 100644 --- a/backend/app/routes/plex/browse.py +++ b/backend/app/routes/plex/browse.py @@ -34,6 +34,7 @@ from app.plex.query import ( apply_meta_filters, apply_movie_state_filter, apply_show_state_filter, + content_libs, field_recheck, genres_col, meta_filter_conds, @@ -47,7 +48,7 @@ from app.plex.query import ( vector_col, vector_match, ) -from app.routes.plex._shared import _content_libs, _norm_lang +from app.routes.plex._shared import _norm_lang from app.utils import to_tsquery_str router = APIRouter(prefix="/api/plex", tags=["plex"]) @@ -109,7 +110,7 @@ def unified_library( offset = max(0, offset) lang = _norm_lang(lang) p = f.as_dict() - movie_lib_ids, show_lib_ids = _content_libs(db) + movie_lib_ids, show_lib_ids = content_libs(db) want_movies = scope in ("movie", "both") and bool(movie_lib_ids) want_shows = scope in ("show", "both") and bool(show_lib_ids) @@ -328,7 +329,7 @@ def facets( "duration_min": None, "duration_max": None, } - all_movie_ids, all_show_ids = _content_libs(db) + all_movie_ids, all_show_ids = content_libs(db) movie_ids = all_movie_ids if scope in ("movie", "both") else [] show_ids = all_show_ids if scope in ("show", "both") else [] if not movie_ids and not show_ids: diff --git a/backend/app/routes/plex/collections.py b/backend/app/routes/plex/collections.py index 308ccb68..862f5f9a 100644 --- a/backend/app/routes/plex/collections.py +++ b/backend/app/routes/plex/collections.py @@ -19,8 +19,7 @@ from app.plex import sync as plex_sync from app.plex.cards import movie_card, show_card from app.plex.client import PlexClient, PlexError, PlexNotConfigured from app.plex.lookups import by_rating_key -from app.plex.query import like_escape, runtime_by_key, top_art_keys -from app.routes.plex._shared import _content_libs +from app.plex.query import content_libs, like_escape, runtime_by_key, top_art_keys # The most-restrictive → least ordering used to pick a collection's effective content rating from its # members (Plex shows one cert on the collection; we don't store it, so derive it). @@ -128,7 +127,7 @@ def collections( # members (a collection lives in one library, so exactly one side has art) — a show collection would # otherwise render blank; the collection's own poster is the next fallback, then the colour hash. # Runtime is movie-only (shows have no single duration). - movie_ids, show_ids = _content_libs(db) + movie_ids, show_ids = content_libs(db) keys = [c.rating_key for c in rows] art_m = top_art_keys(db, PlexItem, movie_ids, PlexItem.collection_keys, keys, per=2) art_s = top_art_keys(db, PlexShow, show_ids, PlexShow.collection_keys, keys, per=2) diff --git a/backend/app/routes/plex/discover.py b/backend/app/routes/plex/discover.py index 68c88937..3bf13054 100644 --- a/backend/app/routes/plex/discover.py +++ b/backend/app/routes/plex/discover.py @@ -27,12 +27,13 @@ from app.plex.query import ( LibraryFilters, apply_meta_filters, apply_show_state_filter, + content_libs, genres_col, meta_filter_conds, top_art_keys, ) from app.plex.tmdb import resolve_recs -from app.routes.plex._shared import _content_libs, _norm_lang +from app.routes.plex._shared import _norm_lang router = APIRouter(prefix="/api/plex", tags=["plex"]) @@ -43,7 +44,7 @@ SURPRISE_PER_KIND = 3 # Surprise-me: at most this many movies + this many shows def _scope_libs(db: Session, scope: str) -> tuple[list[int], list[int]]: """The movie/show library ids visible for this scope (empty list = out of scope).""" - movie_ids, show_ids = _content_libs(db) + movie_ids, show_ids = content_libs(db) return ( movie_ids if scope in ("movie", "both") else [], show_ids if scope in ("show", "both") else [], @@ -112,10 +113,11 @@ def _recently_added( else [] ) # Merge the two type-homogeneous lists by added_at, then serialize each side in one batch so the - # per-card state lookups stay batched (no N+1). The `(is not None, value)` key sorts a missing - # timestamp last under reverse. + # per-card state lookups stay batched (no N+1). Same `or NO_TS` key as the two rails below — the + # E24 review found this one still on the older `(is not None, value)` form, i.e. one rule in two + # spellings, with the two comments explaining it differently. tagged = [(m.added_at, "movie", m) for m in movies] + [(s.added_at, "show", s) for s in shows] - tagged.sort(key=lambda t: (t[0] is not None, t[0]), reverse=True) + tagged.sort(key=lambda t: t[0] or rollup.NO_TS, reverse=True) top = tagged[:RAIL_LIMIT] # Key the serialized cards back by rating_key (globally unique in Plex — a movie and a show never # share one), so the merge doesn't depend on object identity staying alive. @@ -170,7 +172,7 @@ def _recently_watched( ) for ep, st, sh_title, sh_rk in db.execute(eq).all(): rows.append((st.watched_at, leaf_card(ep, st, show_title=sh_title, show_rk=sh_rk))) - rows.sort(key=lambda r: (r[0] is not None, r[0]), reverse=True) + rows.sort(key=lambda r: r[0] or rollup.NO_TS, reverse=True) return [card for _, card in rows[:RAIL_LIMIT]] @@ -234,7 +236,7 @@ def _continue_watching( for sh, resumed_at in srows: rows.append((resumed_at, show_card(sh, "in_progress"))) - rows.sort(key=lambda r: (r[0] is not None, r[0]), reverse=True) + rows.sort(key=lambda r: r[0] or rollup.NO_TS, reverse=True) return [card for _, card in rows[:RAIL_LIMIT]] diff --git a/backend/app/routes/plex/people.py b/backend/app/routes/plex/people.py index 9c1e3db8..ccef7c92 100644 --- a/backend/app/routes/plex/people.py +++ b/backend/app/routes/plex/people.py @@ -19,8 +19,7 @@ from app.models import ( from app.plex import rollup from app.plex import tmdb as plex_tmdb from app.plex.cards import movie_card, show_card -from app.plex.query import split_csv -from app.routes.plex._shared import _content_libs +from app.plex.query import content_libs, split_csv router = APIRouter(prefix="/api/plex", tags=["plex"]) log = logging.getLogger("siftlode.plex") @@ -71,7 +70,7 @@ def people_cards( wanted = split_csv(names)[:16] if not wanted: return {"people": []} - movie_lib_ids, show_lib_ids = _content_libs(db) + movie_lib_ids, show_lib_ids = content_libs(db) want_movies = scope in ("movie", "both") and bool(movie_lib_ids) want_shows = scope in ("show", "both") and bool(show_lib_ids) photos = { @@ -185,7 +184,7 @@ def person_detail( and _should_schedule_enrich(person.id) ): background_tasks.add_task(_enrich_person_bg, person.id) - movie_lib_ids, show_lib_ids = _content_libs(db) + movie_lib_ids, show_lib_ids = content_libs(db) movies: list[dict] = [] if movie_lib_ids: diff --git a/backend/tests/test_discover.py b/backend/tests/test_discover.py new file mode 100644 index 00000000..261001b7 --- /dev/null +++ b/backend/tests/test_discover.py @@ -0,0 +1,942 @@ +"""E24 S1 — the Home surface's read-model. + +Covers the four things that can silently rot here: the time-budget bands (pure), the cross-source +resume rail's merge order, the hero's tier ladder, and the comfort shelf (its schema-level one-of +invariant, its cascade, and the resolved `next` that makes one click enough). + +DB lane (`db` fixture) except for the budget block — skipped without Postgres. +""" + +from datetime import datetime, timedelta, timezone + +import pytest +from sqlalchemy import delete, func, select, text +from sqlalchemy.exc import IntegrityError + +from app.discover import hero, pins, queries, rails +from app.discover.budget import BANDS, fits, remaining_seconds +from app.plex import rollup +from app.plex.cards import leaf_card +from app.models import ( + BlockedChannel, + Channel, + ComfortPin, + PlexItem, + PlexLibrary, + PlexShow, + PlexState, + Playlist, + PlaylistItem, + Subscription, + User, + Video, + VideoState, +) + +NOW = datetime(2026, 6, 1, tzinfo=timezone.utc) + + +# ── budget (pure) ───────────────────────────────────────────────────────────────────────────────── + + +def test_bands_cover_every_runtime_and_overlap_deliberately(): + """Every runtime has a home in `any`, and the bands overlap so "30 perc" is not answered with a + two-minute Short just because it fits under the cap.""" + assert fits(3, "any") and fits(10_000, "any") + assert fits(2 * 60, "short") and not fits(2 * 60, "medium") # a Short is not a 30-minute answer + assert fits(30 * 60, "medium") and not fits(30 * 60, "short") + assert fits(110 * 60, "long") and not fits(110 * 60, "medium") + + +def test_unknown_duration_only_fits_the_unbounded_band(): + """We cannot promise an unknown-length item finishes inside ten minutes, so it is not offered as + an answer to a bounded question — the one control on the page has to mean what it says.""" + for budget in BANDS: + assert fits(None, budget) is (budget == "any") + + +def test_budget_is_matched_against_what_is_LEFT(): + """A 90-minute film with 8 minutes to go is a ten-minute answer.""" + assert remaining_seconds(90 * 60, 82 * 60) == 8 * 60 + assert fits(remaining_seconds(90 * 60, 82 * 60), "short") + assert not fits(remaining_seconds(90 * 60, 0), "short") + assert remaining_seconds(None, 30) is None + assert remaining_seconds(600, 900) == 0 # a position past the end never goes negative + + +# ── fixtures ────────────────────────────────────────────────────────────────────────────────────── + + +def _seed_yt(db) -> tuple[User, Channel]: + """A subscribed channel with three videos of distinct lengths, so a budget can pick between them.""" + user = User(email="discover-e24@example.com") + ch = Channel(id="UCe24", title="Comfort Channel", handle="@comfort") + db.add_all([user, ch]) + db.flush() + db.add(Subscription(user_id=user.id, channel_id=ch.id)) + db.add_all( + [ + Video( + id="vshort", + channel_id=ch.id, + title="Ten minute video", + duration_seconds=9 * 60, + published_at=NOW - timedelta(days=1), + ), + Video( + id="vmed", + channel_id=ch.id, + title="Half hour video", + duration_seconds=30 * 60, + published_at=NOW - timedelta(days=2), + ), + Video( + id="vlong", + channel_id=ch.id, + title="Feature length video", + duration_seconds=95 * 60, + published_at=NOW - timedelta(days=3), + ), + ] + ) + db.commit() + return user, ch + + +def _seed_plex(db, user: User) -> tuple[PlexItem, PlexShow, PlexItem]: + lib_m = PlexLibrary(plex_key="90", title="Movies", kind="movie", enabled=True) + lib_s = PlexLibrary(plex_key="91", title="Shows", kind="show", enabled=True) + db.add_all([lib_m, lib_s]) + db.flush() + movie = PlexItem( + rating_key="pm1", + library_id=lib_m.id, + kind="movie", + title="A Movie", + duration_s=100 * 60, + playable="direct", + ) + show = PlexShow(rating_key="ps1", library_id=lib_s.id, title="A Show", child_count=1) + db.add_all([movie, show]) + db.flush() + ep1 = PlexItem( + rating_key="ps1e1", + library_id=lib_s.id, + kind="episode", + title="Ep 1", + show_id=show.id, + season_number=1, + episode_number=1, + duration_s=45 * 60, + ) + ep2 = PlexItem( + rating_key="ps1e2", + library_id=lib_s.id, + kind="episode", + title="Ep 2", + show_id=show.id, + season_number=1, + episode_number=2, + duration_s=45 * 60, + ) + db.add_all([ep1, ep2]) + db.commit() + return movie, show, ep1 + + +def _enable_plex(db): + from app import sysconfig + + sysconfig.set_value(db, "plex_enabled", True) + db.commit() + + +# ── continue watching ───────────────────────────────────────────────────────────────────────────── + + +def test_continue_watching_merges_both_sources_by_recency(db): + """One rail, not two lists side by side — and ordered by when the user last touched each thing, + across sources. Two side-by-side rails would just move the decision up a level.""" + user, _ = _seed_yt(db) + movie, _, _ = _seed_plex(db, user) + _enable_plex(db) + db.add_all( + [ + VideoState( + user_id=user.id, + video_id="vmed", + status="new", + position_seconds=300, + progress_updated_at=NOW - timedelta(hours=5), + ), + PlexState( + user_id=user.id, + item_id=movie.id, + status="new", + position_seconds=600, + progress_updated_at=NOW - timedelta(hours=1), + ), + ] + ) + db.commit() + + got = rails.continue_watching(db, user) + assert [(c["source"], c["id"]) for c in got] == [("plex", "pm1"), ("youtube", "vmed")] + assert got[0]["remaining_seconds"] == 100 * 60 - 600 + + +def test_continue_watching_is_youtube_only_when_plex_is_off(db): + """Plex is a deployment toggle; with it off the surface says nothing about Plex at all.""" + user, _ = _seed_yt(db) + movie, _, _ = _seed_plex(db, user) + db.add_all( + [ + VideoState( + user_id=user.id, + video_id="vmed", + status="new", + position_seconds=300, + progress_updated_at=NOW, + ), + PlexState( + user_id=user.id, + item_id=movie.id, + status="new", + position_seconds=600, + progress_updated_at=NOW, + ), + ] + ) + db.commit() + assert {c["source"] for c in rails.continue_watching(db, user)} == {"youtube"} + + +def test_the_resume_rail_obeys_the_channel_blocklist(db): + """Blocking a channel does not touch `video_states`, so a half-watched video from it survives + with its resume position — and this rail builds its own SELECT instead of going through + `filtered_query`. "Never show me this channel" has to outrank "you started it", or the landing + page answers "what should I watch" with the thing the user has just banned.""" + user, ch = _seed_yt(db) + db.add( + VideoState( + user_id=user.id, + video_id="vmed", + status="new", + position_seconds=300, + progress_updated_at=NOW, + ) + ) + db.commit() + assert [c["id"] for c in rails.continue_watching(db, user)] == ["vmed"] + + db.add(BlockedChannel(user_id=user.id, channel_id=ch.id)) + db.commit() + assert rails.continue_watching(db, user) == [] + # And so the hero, whose resume tier draws from that rail, has nothing to offer either. + assert hero.pick(db, user, "any", 0) is None + + +def test_a_null_timestamp_sorts_last(db): + """A missing progress timestamp sorts last rather than 500ing the landing page. + + Narrower than it was written to be, and the epic-end review corrected it: the previous key, + `(ts is not None, ts)`, does NOT raise on two nulls — tuple comparison finds the first UNEQUAL + element via `==`, so `None < None` is never reached, and this test passes under that key too. + What it does discriminate is the naive `key=lambda r: r[0]`, which really does raise. See + `plex.rollup.NO_TS` for the corrected rationale.""" + user, _ = _seed_yt(db) + db.add_all( + [ + VideoState( + user_id=user.id, video_id="vmed", status="new", position_seconds=1 + ), # no progress_updated_at + VideoState( + user_id=user.id, video_id="vlong", status="new", position_seconds=1 + ), # nor here + VideoState( + user_id=user.id, + video_id="vshort", + status="new", + position_seconds=1, + progress_updated_at=NOW, + ), + ] + ) + db.commit() + got = rails.continue_watching(db, user) + assert got[0]["id"] == "vshort" + assert len(got) == 3 + + +# ── hero ────────────────────────────────────────────────────────────────────────────────────────── + + +def test_hero_prefers_resume_and_labels_the_tier(db): + user, _ = _seed_yt(db) + db.add( + VideoState( + user_id=user.id, + video_id="vmed", + status="new", + position_seconds=300, + progress_updated_at=NOW, + ) + ) + db.commit() + got = hero.pick(db, user, "any", seed=1) + assert got["tier"] == "resume" + assert got["card"]["id"] == "vmed" + + +def test_hero_skips_a_resume_that_does_not_fit_the_budget(db): + """ "I have ten minutes" must not be answered with the 80 minutes left on a film, even though + that film is the most recent thing the user touched.""" + user, _ = _seed_yt(db) + db.add( + VideoState( + user_id=user.id, + video_id="vlong", + status="new", + position_seconds=60, + progress_updated_at=NOW, + ) + ) + db.commit() + got = hero.pick(db, user, "short", seed=1) + assert got["tier"] == "surprise" + assert got["card"]["duration_seconds"] <= BANDS["short"][1] + + +def test_hero_falls_back_to_the_shelf_before_guessing(db): + """The user's own declared fallback beats anything we could infer.""" + user, ch = _seed_yt(db) + db.add(ComfortPin(user_id=user.id, video_id="vmed", position=0)) + db.commit() + got = hero.pick(db, user, "medium", seed=1) + assert got["tier"] == "comfort" + assert got["card"]["id"] == "vmed" + + +def test_hero_resolves_a_show_to_the_episode_that_would_play(db): + """A show has no Play button. The hero of a partially-watched show is its next episode.""" + user, _ = _seed_yt(db) + _, show, ep1 = _seed_plex(db, user) + _enable_plex(db) + db.add( + PlexState( + user_id=user.id, + item_id=ep1.id, + status="watched", + watched_at=NOW, + progress_updated_at=NOW, + ) + ) + db.commit() + got = hero.pick(db, user, "any", seed=1) + assert got["tier"] == "resume" + assert got["card"]["kind"] == "episode" + assert got["card"]["id"] == "ps1e2" # ep1 is finished → ep2 is what plays + + +def test_hero_is_none_when_nothing_fits(db): + """An empty answer is an honest answer: the page renders its own empty state rather than being + handed a card that does not answer what was asked. Here the whole library is under 40 minutes, + so "egy este" has nothing to offer.""" + user, ch = _seed_yt(db) + db.execute(text("DELETE FROM videos WHERE id = 'vlong'")) + db.commit() + assert hero.pick(db, user, "long", seed=1) is None + + +def test_surprise_is_reproducible_for_a_seed(db): + """The reroll button depends on this: one seed drives both sources, so the same seed must give + the same answer and a different seed must be free to give another.""" + user, _ = _seed_yt(db) + a = rails.surprise(db, user, "any", seed=7, limit=3) + b = rails.surprise(db, user, "any", seed=7, limit=3) + assert [c["id"] for c in a] == [c["id"] for c in b] + seeds = {tuple(c["id"] for c in rails.surprise(db, user, "any", s, 3)) for s in range(12)} + assert len(seeds) > 1 # the seed actually reaches the ordering + + +def test_surprise_never_offers_something_already_started(db): + """The resume tier may have skipped an item because its REMAINING time did not fit; handing the + same item back as "Meglepetés" — where the full runtime does fit — would contradict the question + the user just answered.""" + user, _ = _seed_yt(db) + db.add( + VideoState( + user_id=user.id, + video_id="vlong", + status="new", + position_seconds=90 * 60, + progress_updated_at=NOW, + ) + ) + db.commit() + assert "vlong" not in {c["id"] for c in rails.surprise(db, user, "long", seed=3, limit=5)} + + +def test_the_seed_decides_which_source_leads(db): + """With a pool of one, a fixed source order would mean the Plex library could never answer "egy + este" whenever a YouTube video also fit — the cross-source promise would quietly be a YouTube + promise.""" + user, _ = _seed_yt(db) + _seed_plex(db, user) + _enable_plex(db) + leaders = {rails.surprise(db, user, "any", s, limit=1)[0]["source"] for s in range(6)} + assert leaders == {"youtube", "plex"} + + +def test_the_reroll_must_be_able_to_move_past_the_resume_tier(db): + """A new seed alone only re-draws the SURPRISE tier — the resume tier deterministically returns + the most recently progressed thing that fits, so without an exclusion the reroll handed the same + card straight back and the button looked dead (found in the browser, 2026-09-06).""" + user, _ = _seed_yt(db) + db.add_all( + [ + VideoState( + user_id=user.id, + video_id="vmed", + status="new", + position_seconds=300, + progress_updated_at=NOW, + ), + VideoState( + user_id=user.id, + video_id="vshort", + status="new", + position_seconds=60, + progress_updated_at=NOW - timedelta(hours=1), + ), + ] + ) + db.commit() + + first = hero.pick(db, user, "any", seed=1) + assert first["card"]["id"] == "vmed" + # Same seed, same everything — only the exclusion differs. + second = hero.pick(db, user, "any", seed=1, exclude={"youtube:vmed"}) + assert second["card"]["id"] == "vshort" + + +def test_the_reroll_rotates_which_tier_answers(db): + """Measured 2026-09-06: exclusion alone left the button walking down the resume rail (20 rows) + and cycling nine cards, every one of them `resume` — the shelf and the surprise tier were + unreachable by pressing it. `start` is the press count, so consecutive presses offer a different + KIND of answer.""" + user, _ = _seed_yt(db) + # One of each: something half-finished, something pinned, and a third video for the draw. + db.add( + VideoState( + user_id=user.id, + video_id="vmed", + status="new", + position_seconds=300, + progress_updated_at=NOW, + ) + ) + db.add(ComfortPin(user_id=user.id, video_id="vlong", position=0)) + db.commit() + tiers = [hero.pick(db, user, "any", seed=1, start=n)["tier"] for n in range(4)] + assert tiers == ["resume", "comfort", "surprise", "resume"] + + +def test_the_rotation_still_falls_through_an_empty_tier(db): + """A press must not come back empty just because the tier it happened to lead with has nothing. + Here there is no shelf at all, so the comfort-led press has to answer from somewhere else.""" + user, _ = _seed_yt(db) + assert db.execute(select(func.count()).select_from(ComfortPin)).scalar_one() == 0 + got = hero.pick(db, user, "any", seed=1, start=1) + assert got is not None and got["tier"] != "comfort" + + +def test_excluding_a_show_also_excludes_the_episode_it_resolves_to(db): + """The user rerolled past the SHOW they were shown; handing back its next episode under a + different title would be the same suggestion wearing a hat.""" + user, _ = _seed_yt(db) + _, show, ep1 = _seed_plex(db, user) + _enable_plex(db) + db.add( + PlexState( + user_id=user.id, + item_id=ep1.id, + status="new", + position_seconds=120, + progress_updated_at=NOW, + ) + ) + db.commit() + + shown = hero.pick(db, user, "any", seed=1) + assert shown["card"]["id"] == "ps1e1" # the show resolved to its in-progress episode + # Excluding what the USER saw — the show — must move the tier on, not return the episode again. + after = hero.pick(db, user, "any", seed=1, exclude={"plex:ps1"}) + assert after is None or after["card"]["id"] != "ps1e1" + + +# ── comfort shelf ───────────────────────────────────────────────────────────────────────────────── + + +def test_a_pin_must_reference_exactly_one_thing(db): + """The union invariant lives in the schema, not in a comment at every insert site.""" + user, ch = _seed_yt(db) + for kwargs in ({}, {"video_id": "vmed", "channel_id": ch.id}): + db.add(ComfortPin(user_id=user.id, position=0, **kwargs)) + with pytest.raises(IntegrityError): + db.flush() + db.rollback() + + +def test_the_same_thing_cannot_be_pinned_twice(db): + """The per-kind unique constraints are what make the route's concurrent-tab path safe (it catches + `IntegrityError` precisely because of them) — and the migration drift check filters constraint + diffs out as noise, so nothing else would notice them going missing. The route's own dedupe + returns early without ever reaching the DB, so only this test exercises them.""" + user, _ = _seed_yt(db) + db.add(ComfortPin(user_id=user.id, video_id="vmed", position=0)) + db.commit() + db.add(ComfortPin(user_id=user.id, video_id="vmed", position=1)) + with pytest.raises(IntegrityError): + db.flush() + db.rollback() + + +def test_deleting_the_pinned_row_removes_the_pin(db): + """A film dropped from the library must not leave a dead tile on the one surface whose whole job + is to be reliable — which is why the shelf holds foreign keys, not strings.""" + user, _ = _seed_yt(db) + db.add(ComfortPin(user_id=user.id, video_id="vmed", position=0)) + db.commit() + db.execute(text("DELETE FROM videos WHERE id = 'vmed'")) + db.commit() + assert db.execute(select(func.count()).select_from(ComfortPin)).scalar() == 0 + + +def test_shelf_resolves_a_channel_to_its_newest_unwatched_video(db): + """ "Put that channel on" has to be one click, so the tile carries what would play.""" + user, ch = _seed_yt(db) + db.add(ComfortPin(user_id=user.id, channel_id=ch.id, position=0)) + db.commit() + (card,) = pins.shelf(db, user) + assert card["kind"] == "channel" + assert card["unwatched_count"] == 3 + assert card["next"]["id"] == "vshort" # newest + + +def test_shelf_resolves_a_playlist_to_its_first_unwatched_item(db): + user, _ = _seed_yt(db) + pl = Playlist(user_id=user.id, name="Elalvós") + db.add(pl) + db.flush() + db.add_all( + [ + PlaylistItem(playlist_id=pl.id, video_id="vshort", position=0), + PlaylistItem(playlist_id=pl.id, video_id="vmed", position=1), + ] + ) + db.add(VideoState(user_id=user.id, video_id="vshort", status="watched", watched_at=NOW)) + db.add(ComfortPin(user_id=user.id, playlist_id=pl.id, position=0)) + db.commit() + (card,) = pins.shelf(db, user) + assert card["kind"] == "playlist" + assert card["item_count"] == 2 + assert card["next"]["id"] == "vmed" + + +def test_a_pinned_video_survives_leaving_the_feed_scope(db): + """The shelf is the fallback surface. An entry silently vanishing because the user unsubscribed + from the channel is exactly the failure it must not have.""" + user, ch = _seed_yt(db) + db.add(ComfortPin(user_id=user.id, video_id="vmed", position=0)) + db.commit() + db.execute(delete(Subscription)) + db.commit() + (card,) = pins.shelf(db, user) + assert card["id"] == "vmed" + assert card["next"]["id"] == "vmed" + + +def test_the_plex_surprise_obeys_the_time_budget(db): + """The one control on the page has to mean what it says: a 100-minute film is not an answer to + "10 perc". The YouTube half of this rule has had its own test since S1; the Plex half's duration + bounds could be deleted with the whole suite still green, because every test that reached + `plex_surprise` passed budget="any".""" + user, _ = _seed_yt(db) + _seed_plex(db, user) + _enable_plex(db) + drawn = lambda budget: { # noqa: E731 + (c["source"], c["id"]) for s in range(6) for c in rails.surprise(db, user, budget, s, 5) + } + assert ("plex", "pm1") not in drawn("short") + # …and it IS offered where it fits, so this cannot pass by the rail being empty. + assert ("plex", "pm1") in drawn("long") + + +def test_the_plex_surprise_never_offers_something_already_started(db): + """A half-watched film is not a surprise, and the resume tier has already had its chance at it. + The mirror of `test_surprise_never_offers_something_already_started` for the other source — the + asymmetric half of one principle, which the house rules say to fix everywhere.""" + user, _ = _seed_yt(db) + movie, _, _ = _seed_plex(db, user) + _enable_plex(db) + db.add( + PlexState( + user_id=user.id, + item_id=movie.id, + status="new", + position_seconds=600, + progress_updated_at=NOW, + ) + ) + db.commit() + drawn = { + (c["source"], c["id"]) for s in range(6) for c in rails.surprise(db, user, "long", s, 5) + } + assert ("plex", "pm1") not in drawn + + +def test_a_new_pin_lands_at_the_end_even_after_a_hole(db): + """Positions are never renumbered on removal — `remove_pin` deletes the row, and every FK cascade + silently removes a pin when its target disappears — so the append position has to come from + `max(position)`, not from the row count. With a count, the first hole drops the next pin into the + MIDDLE of the shelf and the second makes two pins claim one slot.""" + user, ch = _seed_yt(db) + db.add_all( + [ + ComfortPin(user_id=user.id, video_id="vshort", position=0), + ComfortPin(user_id=user.id, video_id="vmed", position=1), + ComfortPin(user_id=user.id, video_id="vlong", position=2), + ] + ) + db.commit() + db.execute(delete(ComfortPin).where(ComfortPin.video_id.in_(("vshort", "vmed")))) + db.commit() + + db.add(ComfortPin(user_id=user.id, channel_id=ch.id, position=pins.next_position(db, user))) + db.commit() + assert [c["kind"] for c in pins.shelf(db, user)] == ["video", "channel"] + positions = ( + db.execute(select(ComfortPin.position).order_by(ComfortPin.position)).scalars().all() + ) + assert len(set(positions)) == len(positions) # and no two tiles claim the same slot + + +def test_a_self_playing_pin_does_not_reference_itself(db): + """`next` must be a distinct dict even when the pin plays itself. Aliasing the card into its own + `next` is a cycle, and the serializer turns the whole shelf into a 500 over it.""" + user, _ = _seed_yt(db) + db.add(ComfortPin(user_id=user.id, video_id="vmed", position=0)) + db.commit() + (card,) = pins.shelf(db, user) + assert card["next"] is not card + assert "next" not in card["next"] + + +def test_the_resume_rail_says_what_would_actually_play(db): + """A show tile that only opens the show page moves the decision instead of removing it — the + click lands on six seasons to choose between, which is the paralysis this whole surface exists + for. The shelf and the hero already resolved; this rail did not (reported 2026-09-06). The tile + stays the show (its artwork is what the user recognises); `next` is what starts.""" + user, _ = _seed_yt(db) + _, show, ep1 = _seed_plex(db, user) + _enable_plex(db) + db.add( + PlexState( + user_id=user.id, + item_id=ep1.id, + status="watched", + watched_at=NOW, + progress_updated_at=NOW, + ) + ) + db.commit() + rail = rails.continue_watching(db, user) + tile = next(c for c in rail if c["kind"] == "show") + assert tile["id"] == "ps1" # still the show + assert tile["next"]["kind"] == "episode" + assert tile["next"]["id"] == "ps1e2" # ep1 is finished → ep2 is what plays + + +def test_only_a_show_row_pays_for_a_resolution(db): + """`next` is the answer to "a show has no Play button". Attaching it to a movie or a video would + both cost a query for nothing and re-open the self-reference cycle the shelf already hit. + + The states are what make this test exist: without them every rail query inner-joins its way to + zero rows, the loop body never runs, and the assertion is vacuous — which is exactly how the + epic-end review found it.""" + user, _ = _seed_yt(db) + movie, _, ep1 = _seed_plex(db, user) + _enable_plex(db) + db.add_all( + [ + VideoState( + user_id=user.id, + video_id="vmed", + status="new", + position_seconds=300, + progress_updated_at=NOW, + ), + PlexState( + user_id=user.id, + item_id=movie.id, + status="new", + position_seconds=600, + progress_updated_at=NOW, + ), + PlexState( + user_id=user.id, + item_id=ep1.id, + status="new", + position_seconds=120, + progress_updated_at=NOW, + ), + ] + ) + db.commit() + + rail = rails.continue_watching(db, user) + # A video, a movie and a show — one row of each kind that can appear here, so the assertion below + # actually exercises both sides of the equality. + assert {c["kind"] for c in rail} == {"video", "movie", "show"} + for card in rail: + assert ("next" in card) == (card["kind"] == "show"), card["id"] + + +def test_next_episode_takes_an_episode_with_no_state_row_at_all(db): + """The NULL trap in the SQL rewrite: `status NOT IN ('watched','hidden')` is UNKNOWN — not true — + for an episode the user has never touched, so a bare filter drops exactly the common case and the + rail would offer episode 1 of a show already half-watched.""" + user, _ = _seed_yt(db) + _, show, ep1 = _seed_plex(db, user) + db.add(PlexState(user_id=user.id, item_id=ep1.id, status="watched", watched_at=NOW)) + db.commit() # ep2 has NO PlexState row + assert queries.next_episode(db, user.id, show.id)["id"] == "ps1e2" + + +def test_a_finished_show_starts_again_from_the_top(db): + """Nothing unwatched left is not "nothing to play" — it is "watch it again".""" + user, _ = _seed_yt(db) + _, show, ep1 = _seed_plex(db, user) + ep2 = db.execute(select(PlexItem).where(PlexItem.rating_key == "ps1e2")).scalar_one() + db.add_all( + [ + PlexState(user_id=user.id, item_id=ep1.id, status="watched", watched_at=NOW), + PlexState(user_id=user.id, item_id=ep2.id, status="watched", watched_at=NOW), + ] + ) + db.commit() + assert queries.next_episode(db, user.id, show.id)["id"] == "ps1e1" + + +def test_next_episode_agrees_with_the_show_page_when_two_are_in_progress(db): + """One show, one answer. `plex.rollup.rollup` — which drives the show page's Resume button — + takes the LAST in-progress episode in broadcast order; taking the FIRST here gave the same show + two different "continue" answers, with the rail ranking it by its most recent touch and then + offering the episode abandoned weeks earlier. Two in-progress episodes are not exotic: a scrub to + the end leaves a stale `status='new', position>0` row until the daily reconcile.""" + user, _ = _seed_yt(db) + _, show, ep1 = _seed_plex(db, user) + ep2 = db.execute(select(PlexItem).where(PlexItem.rating_key == "ps1e2")).scalar_one() + db.add_all( + [ + PlexState( + user_id=user.id, + item_id=ep1.id, + status="new", + position_seconds=300, + progress_updated_at=NOW - timedelta(days=9), + ), + PlexState( + user_id=user.id, + item_id=ep2.id, + status="new", + position_seconds=600, + progress_updated_at=NOW, + ), + ] + ) + db.commit() + + got = queries.next_episode(db, user.id, show.id) + assert got["id"] == "ps1e2" + # Asserted against the other implementation rather than against a literal, so the two cannot + # drift apart again without this failing. + ordered = [ + leaf_card(ep, db.execute(select(PlexState).where(PlexState.item_id == ep.id)).scalar_one()) + for ep in (ep1, ep2) + ] + assert rollup.rollup(ordered)["resume"]["id"] == got["id"] + + +def test_next_episode_prefers_the_one_in_progress(db): + """In progress beats "next unwatched": resuming is cheaper than starting.""" + user, _ = _seed_yt(db) + _, show, ep1 = _seed_plex(db, user) + ep2 = db.execute(select(PlexItem).where(PlexItem.rating_key == "ps1e2")).scalar_one() + db.add_all( + [ + PlexState(user_id=user.id, item_id=ep1.id, status="watched", watched_at=NOW), + PlexState(user_id=user.id, item_id=ep2.id, status="new", position_seconds=120), + ] + ) + db.commit() + assert queries.next_episode(db, user.id, show.id)["id"] == "ps1e2" + + +# ── HTTP surface ────────────────────────────────────────────────────────────────────────────────── + + +def test_discover_endpoint_answers_in_one_pass(db, as_user): + user, _ = _seed_yt(db) + db.add( + VideoState( + user_id=user.id, + video_id="vmed", + status="new", + position_seconds=300, + progress_updated_at=NOW, + ) + ) + db.commit() + r = as_user(user).get("/api/discover?budget=medium") + assert r.status_code == 200 + body = r.json() + assert body["budget"] == "medium" + assert body["hero"]["tier"] == "resume" + assert [c["id"] for c in body["continue_watching"]] == ["vmed"] + assert body["comfort"] == [] + + +def test_an_unknown_budget_is_refused_rather_than_silently_widened(db, as_user): + user, _ = _seed_yt(db) + assert as_user(user).get("/api/discover?budget=whenever").status_code == 422 + + +def test_pinning_twice_is_the_same_shelf(db, as_user): + """The gesture is "make sure this is on my shelf", so a re-pin is a no-op, not a 409.""" + user, _ = _seed_yt(db) + c = as_user(user) + first = c.post("/api/discover/pins", json={"kind": "video", "ref": "vmed"}).json() + second = c.post("/api/discover/pins", json={"kind": "video", "ref": "vmed"}).json() + assert first["created"] is True and second["created"] is False + assert first["id"] == second["id"] + assert len(c.get("/api/discover/pins").json()["items"]) == 1 + + +def test_pinning_a_plex_show_resolves_to_the_episode_that_would_play(db, as_user): + """Two of the five pinnable kinds are Plex, and the rating_key → internal-id translation is + theirs alone — a wrong column or a wrong model in either arm pins the wrong row, or 404s every + Plex pin. Neither arm had a test before the epic-end review.""" + user, _ = _seed_yt(db) + _, _, ep1 = _seed_plex(db, user) + _enable_plex(db) + db.add(PlexState(user_id=user.id, item_id=ep1.id, status="watched", watched_at=NOW)) + db.commit() + c = as_user(user) + assert c.post("/api/discover/pins", json={"kind": "plex_show", "ref": "ps1"}).status_code == 200 + (tile,) = c.get("/api/discover/pins").json()["items"] + assert (tile["kind"], tile["id"]) == ("show", "ps1") + assert tile["next"]["id"] == "ps1e2" # ep1 is finished + + +def test_pinning_a_plex_movie_plays_itself_without_a_cycle(db, as_user): + """The self-reference that once 500'd the whole shelf, on the other leaf kind: `next` has to be a + distinct dict, not the tile aliased into itself. Covered for a video since S1, never for a movie.""" + user, _ = _seed_yt(db) + _seed_plex(db, user) + _enable_plex(db) + c = as_user(user) + assert c.post("/api/discover/pins", json={"kind": "plex_item", "ref": "pm1"}).status_code == 200 + (tile,) = c.get("/api/discover/pins").json()["items"] + assert tile["id"] == "pm1" and tile["next"]["id"] == "pm1" + + +def test_the_reroll_endpoint_parses_its_whole_exclusion_list_and_its_press_count(db, as_user): + """The reroll's behaviour is covered at the domain level; the ROUTE's query-string translation — + the comma split and the `start` passthrough — was not covered at all, so a change there could + hand back the same card on every press with the suite green. Excluding all three candidates is + what proves the whole list parsed: a delimiter change leaves one unsplit string in the set, and a + hero comes back. + + NOT the `MAX_EXCLUDE` clamp, which this sends three keys against a cap of 32 and cannot see. The + docstring used to claim it did (round 2). The clamp is deliberately left unpinned: it guards + nothing the route would otherwise do — see the constant's own comment — and a test that sent 33 + keys could only assert that the request still answers, which the comma split already proves.""" + user, _ = _seed_yt(db) + db.add( + VideoState( + user_id=user.id, + video_id="vmed", + status="new", + position_seconds=300, + progress_updated_at=NOW, + ) + ) + db.add(ComfortPin(user_id=user.id, video_id="vlong", position=0)) + db.commit() + c = as_user(user) + + # `start` rotates which tier answers first: 0 leads with resume, 1 with the shelf. + assert c.get("/api/discover/pick?seed=1&start=0").json()["hero"]["tier"] == "resume" + assert c.get("/api/discover/pick?seed=1&start=1").json()["hero"]["tier"] == "comfort" + + skip = ",".join(f"youtube:{v}" for v in ("vshort", "vmed", "vlong")) + assert c.get(f"/api/discover/pick?seed=2&exclude={skip}").json()["hero"] is None + assert c.get("/api/discover/pick?budget=whenever").status_code == 422 + + +def test_pinning_something_that_does_not_exist_is_a_404(db, as_user): + user, _ = _seed_yt(db) + r = as_user(user).post("/api/discover/pins", json={"kind": "video", "ref": "nope"}) + assert r.status_code == 404 + + +def test_a_pin_whose_target_vanishes_mid_request_is_a_404_not_a_500(db, as_user, monkeypatch): + """`add_pin`'s `except IntegrityError` arm was reached by no test at all, so the 500→404 + narrowing could be reverted with the whole suite green (round 2). The 404 above comes from the + `resolve_ref` PRE-CHECK, a different branch that returns before the INSERT. + + The race the arm exists for — a Plex sync dropping the row between `resolve_ref` confirming it + and the INSERT — needs two transactions, but the BRANCH does not: a `resolve_ref` pointing at an + id no row carries lets the pre-check pass and trips the same FK on commit, with the re-query + finding no winner. The sentinel has to FIT `comfort_pins.video_id` (varchar(16)) — a longer one + raises `DataError` on the way in, which this arm does not catch and never meant to. Its sibling arm (a concurrent duplicate, where the re-query DOES find a winner + and answers 200) genuinely needs a second connection and stays uncovered.""" + user, _ = _seed_yt(db) + monkeypatch.setattr(pins, "resolve_ref", lambda *a, **k: ("video_id", "gone-mid-req")) + r = as_user(user).post("/api/discover/pins", json={"kind": "video", "ref": "vshort"}) + assert r.status_code == 404 + + +def test_another_users_playlist_is_not_pinnable(db, as_user): + """404, not 403 — a 403 would confirm the id exists to a stranger.""" + user, _ = _seed_yt(db) + other = User(email="other-e24@example.com") + db.add(other) + db.flush() + pl = Playlist(user_id=other.id, name="Theirs") + db.add(pl) + db.commit() + r = as_user(user).post("/api/discover/pins", json={"kind": "playlist", "ref": str(pl.id)}) + assert r.status_code == 404 + + +def test_reorder_ignores_ids_that_are_not_yours(db, as_user): + """A stale tab replaying an order that includes a since-deleted pin should reorder what still + exists rather than fail wholesale.""" + user, ch = _seed_yt(db) + c = as_user(user) + a = c.post("/api/discover/pins", json={"kind": "video", "ref": "vmed"}).json()["id"] + b = c.post("/api/discover/pins", json={"kind": "channel", "ref": ch.id}).json()["id"] + assert c.post("/api/discover/pins/reorder", json={"ids": [b, 9999, a]}).status_code == 200 + assert [p["pin_id"] for p in c.get("/api/discover/pins").json()["items"]] == [b, a] + + +def test_deleting_a_pin_reports_what_went(db, as_user): + user, _ = _seed_yt(db) + c = as_user(user) + pin_id = c.post("/api/discover/pins", json={"kind": "video", "ref": "vmed"}).json()["id"] + r = c.delete(f"/api/discover/pins/{pin_id}") + assert r.status_code == 200 and r.json() == {"deleted": pin_id} + assert c.get("/api/discover/pins").json()["items"] == [] diff --git a/backend/tests/test_plex_overlay.py b/backend/tests/test_plex_overlay.py index 3fa4c0c6..5b30d00b 100644 --- a/backend/tests/test_plex_overlay.py +++ b/backend/tests/test_plex_overlay.py @@ -12,7 +12,7 @@ import app.plex.sync as syncmod from app import sysconfig from app.models import PlexItem, PlexLibrary from app.plex.sync import _apply_item, _canonical_guid, _sync_overlay -from app.routes.plex._shared import _content_libs +from app.plex.query import content_libs class _StubPlex: @@ -155,7 +155,7 @@ def test_sync_designates_overlay_and_writes_hu(db, monkeypatch): assert m.title_hu == "Öldöklés" assert m.genres_hu == ["Akció"] # The HU section is not browsable and mirrored no rows of its own. - movie_ids, _ = _content_libs(db) + movie_ids, _ = content_libs(db) assert en.id in movie_ids and hu.id not in movie_ids assert db.scalar(select(func.count(PlexItem.id)).where(PlexItem.library_id == hu.id)) == 0 assert stats["overlaid"] == 1 @@ -240,7 +240,7 @@ def test_content_libs_excludes_overlay_libraries(db): db.add(overlay) db.commit() - movie_ids, show_ids = _content_libs(db) + movie_ids, show_ids = content_libs(db) assert en_lib.id in movie_ids assert overlay.id not in movie_ids # the HU overlay is never browsable assert show_ids == [en_shows.id] diff --git a/backend/tests/test_plex_person.py b/backend/tests/test_plex_person.py index 5daa5809..760aac7c 100644 --- a/backend/tests/test_plex_person.py +++ b/backend/tests/test_plex_person.py @@ -1,6 +1,6 @@ """DB-backed regression (E5 S4): GET /api/plex/person/{name} returns a cast/crew member's filmography — movies + shows in the ENABLED libraries whose cast OR directors include the name, -each carrying the caller's watch-state, plus the cached headshot. Also covers `_content_libs` +each carrying the caller's watch-state, plus the cached headshot. Also covers `content_libs` scoping (disabled libraries excluded) and the kind filter (episodes are not movies). Uses the DB lane (`db` fixture) — skipped when no Postgres is reachable. @@ -20,7 +20,7 @@ from app.models import ( PlexState, User, ) -from app.routes.plex._shared import _content_libs +from app.plex.query import content_libs NAME = "Robert De Niro" @@ -151,5 +151,5 @@ def test_content_libs_partitions_only_enabled(db): ] ) db.commit() - movie_ids, show_ids = _content_libs(db) + movie_ids, show_ids = content_libs(db) assert len(movie_ids) == 1 and len(show_ids) == 1 # the disabled movie lib is excluded diff --git a/frontend/src/App.keymap.contract.test.ts b/frontend/src/App.keymap.contract.test.ts new file mode 100644 index 00000000..ee26db20 --- /dev/null +++ b/frontend/src/App.keymap.contract.test.ts @@ -0,0 +1,33 @@ +// The global Shift+W wiring, pinned at its call site. +// +// What this is: a source contract, the same idiom as uiBoundary.contract.test.ts — a structural rule +// asserted directly rather than left to prose. What it is NOT: a behaviour test. The behaviour lives +// in history.dom.test.ts and history.burial.dom.test.ts, which drive `navigateOverOverlays` against +// real overlay entries; the end-to-end gesture (Shift+W over an open player, then Back) is a browser +// check, because jsdom reproduces history ordering only as bookkeeping. +// +// It exists because the composition line itself was unpinned: App.tsx is the one module no test +// renders (main.tsx lazy-imports it and nothing else does), globalKeymap.dom.test.tsx drives the +// hook with its OWN handler, and history.dom.test.ts drives the helper with its own navigate — so +// dropping `navigateOverOverlays` from App left all 818 tests green while the user-visible bug came +// straight back (round 2, verified by running that revert). +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +import { describe, expect, it } from "vitest"; + +const APP = readFileSync(fileURLToPath(new URL("./App.tsx", import.meta.url)), "utf8"); + +describe("App · global keymap wiring", () => { + it("routes the global home shortcut through navigateOverOverlays", () => { + const call = /useGlobalKeymap\(([\s\S]*?)\);/.exec(APP)?.[1]; + // A rename or a move is a legitimate change — but then this test is the thing that has to be + // updated deliberately, which is the whole point of pinning it. + expect( + call, + "App.tsx no longer calls useGlobalKeymap in a shape this test can read", + ).toBeTypeOf("string"); + expect(call).toContain("navigateOverOverlays"); + expect(call).toContain('setPage("home")'); + }); +}); diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 7fdb2b6e..8d9de728 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -37,6 +37,7 @@ import Toaster from "./components/ui/Toaster"; import KeyboardHelp from "./components/ui/KeyboardHelp"; import AppCommandPalette from "./components/AppCommandPalette"; import { useGlobalKeymap } from "./lib/keymap"; +import { navigateOverOverlays } from "./lib/history"; import { StateMessage } from "./components/ui/QueryState"; import ErrorDialog from "./components/ErrorDialog"; import VersionBanner from "./components/VersionBanner"; @@ -137,10 +138,17 @@ export default function App() { refetchInterval: (query) => (query.state.data ? 60_000 : false), }); - // The `global` key scope: `h` opens the cheat sheet, Mod+K the jump-to palette. Bound only once - // signed in, because that is where both overlays are mounted (bottom of this tree) — see - // useGlobalKeymap on why binding it above the signed-out branch is worse than not binding it. - useGlobalKeymap(!!meQuery.data); + // The `global` key scope: `h` opens the cheat sheet, Mod+K the jump-to palette, Shift+W jumps to + // Home ("mit nézzek?"). Bound only once signed in, because that is where the overlays are mounted + // (bottom of this tree) and where the modules exist at all — see useGlobalKeymap on why binding it + // above the signed-out branch is worse than not binding it. + // Through `navigateOverOverlays`, because a global key can fire while an overlay is open and does + // not know it: the page change unmounts the player (it lives inside the module), whose reconcile + // then reclaimed the entry this push had just made — Shift+W over a playing video closed the + // player and snapped straight back to the feed. `setPage`'s boolean is load-bearing here, not + // decoration: pressed while ALREADY on Home (the default landing page) it navigates nowhere, and + // burying the open player's entry under a push that never happened orphaned it — see `buryUnder`. + useGlobalKeymap(!!meQuery.data, () => navigateOverOverlays(() => setPage("home"))); // Pin this tab to whatever account it first loaded as. Without this, a tab that never picked // an account (it was riding the session's default) would silently swap identity when ANOTHER diff --git a/frontend/src/components/AppCommandPalette.tsx b/frontend/src/components/AppCommandPalette.tsx index 500ef172..a4c3f33a 100644 --- a/frontend/src/components/AppCommandPalette.tsx +++ b/frontend/src/components/AppCommandPalette.tsx @@ -1,7 +1,7 @@ import { useMemo } from "react"; import { useTranslation } from "react-i18next"; import { useQuery } from "@tanstack/react-query"; -import { Keyboard, ListVideo, Palette, Tv } from "lucide-react"; +import { Keyboard, ListVideo, Palette, Sparkles, Tv } from "lucide-react"; import CommandPalette, { type CommandGroup } from "./ui/CommandPalette"; import { api } from "../lib/api"; import { qk } from "../lib/queryKeys"; @@ -9,6 +9,7 @@ import { useMe } from "../lib/useMe"; import { moduleDef, moduleOrder } from "../lib/modules"; import { SCHEMES, type Scheme } from "../lib/theme"; import { closePalette, toggleKeyboardHelp, usePaletteOpen } from "../lib/keymap"; +import { requestSurprise } from "../lib/homeIntent"; import { useNavigationActions } from "./NavigationProvider"; import { usePlaylistsActions } from "./PlaylistsProvider"; import { usePrefs } from "./PrefsProvider"; @@ -124,6 +125,21 @@ export default function AppCommandPalette() { id: "actions", label: t("palette.groups.actions"), items: [ + // "Meglepetés" — Home's reroll, reachable from anywhere (E24). There is deliberately no + // second "Mit nézzek?" row beside it: the Home module row above ALREADY matches that phrase, + // because the palette searches a module's descriptive `titleKey` too and Home's is literally + // "Mit nézzek most?". A row that navigates where an existing row already navigates would be + // duplication in the one list whose whole job is to be scanned quickly. + { + id: "action:surprise", + label: t("palette.actions.surprise"), + icon: , + keywords: t("home.title"), + run: () => { + setPage("home"); + requestSurprise(); + }, + }, { id: "action:shortcuts", label: t("keys.title"), diff --git a/frontend/src/components/ChannelLayoutGrid.tsx b/frontend/src/components/ChannelLayoutGrid.tsx index 58544afe..aa44ddd3 100644 --- a/frontend/src/components/ChannelLayoutGrid.tsx +++ b/frontend/src/components/ChannelLayoutGrid.tsx @@ -24,6 +24,9 @@ interface ChannelLayoutGridProps { /** Column key rendered as the action cluster, pinned to the card's bottom edge. */ actionsKey?: string; rowClassName?: (row: T) => string; + /** Extra DOM props per card — same escape hatch, same single consumer, as `DataTable.rowProps`, so + * a channel answers the right-click in the card layout exactly as it does in the table. */ + rowProps?: (row: T) => Record; emptyText?: string; } @@ -35,6 +38,7 @@ export default function ChannelLayoutGrid({ aboutKey, actionsKey, rowClassName, + rowProps, emptyText, }: ChannelLayoutGridProps) { const { t } = useTranslation(); @@ -50,6 +54,7 @@ export default function ChannelLayoutGrid({ const renderCard = (row: T) => (
diff --git a/frontend/src/components/Channels.tsx b/frontend/src/components/Channels.tsx index 1fff9b9c..54799ea6 100644 --- a/frontend/src/components/Channels.tsx +++ b/frontend/src/components/Channels.tsx @@ -15,6 +15,9 @@ import Tooltip from "./ui/Tooltip"; import DataTable from "./ui/DataTable"; import Tabs, { type TabDef } from "./ui/Tabs"; import ChannelLayoutGrid from "./ChannelLayoutGrid"; +import { ComfortPinMenu } from "./ComfortPinMenu"; +import { pinChannel } from "./comfortPin"; +import { useContextMenu } from "./ui/useContextMenu"; import { LoadingState, StateMessage } from "./ui/QueryState"; import { useChannelTagActions } from "../lib/useChannelTagActions"; import { useTagSuggestions } from "./useTagSuggestions"; @@ -412,6 +415,11 @@ export default function Channels() { /> ); const hasLayout = view === "subscribed" || view === "discovery"; + // The comfort-shelf pin on a channel row (E24 S3). ONE menu for the list, shared by the table and + // the card layout — they are two paints of the same rows, so a right-click must mean the same + // thing in both. Only mounted under the SUBSCRIBED branch below: a discovery result is not a + // `Channel` row yet, so pinning one could only 404. + const pinMenu = useContextMenu(); // In cards there are no column headers to sort by, so a dropdown drives the SAME shared sort state // the table header uses (table ↔ cards stay in sync). Table mode keeps the header clicks. const sortControl = ( @@ -613,6 +621,7 @@ export default function Channels() { onFiltersChange={setFilters} fixedLayout emptyText={t("channels.empty")} + rowProps={pinMenu.triggerProps} /> ) : ( )} + pinChannel(c.id)} + title={(c) => c.title ?? t("channels.row.thisChannel")} + />
)} diff --git a/frontend/src/components/ComfortPinMenu.tsx b/frontend/src/components/ComfortPinMenu.tsx new file mode 100644 index 00000000..7a3c5e73 --- /dev/null +++ b/frontend/src/components/ComfortPinMenu.tsx @@ -0,0 +1,50 @@ +import { useTranslation } from "react-i18next"; + +import { type PlexCard, type Video } from "../lib/api"; +import { pinPlex, pinVideo, useComfortPin, type PinTarget } from "./comfortPin"; +import { ContextMenu } from "./ui/ContextMenu"; +import { type ContextMenuController } from "./ui/useContextMenu"; + +// The menu SURFACE for the comfort-shelf pin (E24 S3). Split from `comfortPin.tsx` for the same +// reason `ContextMenu.tsx` is split from `useContextMenu.ts`: a file that exports components and +// nothing else keeps react-refresh working for every surface that imports it. + +/** + * The whole right-click menu for a surface whose only context action is the pin — the card grids and + * rails. Render ONE per list, outside the rows (see `ui/ContextMenu`), and spread + * `menu.triggerProps` on each row. + * + * `target` maps the row the user right-clicked to its pin; `title` names it in the confirmation + * toast. Both come from the call site because only it knows what its rows are. + */ +export function ComfortPinMenu({ + menu, + target, + title, +}: { + menu: ContextMenuController; + target: (row: T) => PinTarget | null; + title: (row: T) => string; +}) { + const { t } = useTranslation(); + const pin = useComfortPin(menu.open); + const row = menu.target; + return ( + + ); +} + +/** The Plex poster grids and rails — five of them, all pinning the same way. One component so the + * show-vs-item decision and the toast's name are written once rather than five times. */ +export function PlexPinMenu({ menu }: { menu: ContextMenuController }) { + return c.title} />; +} + +/** The YouTube video lists (the feed grid, the Home rails). */ +export function VideoPinMenu({ menu }: { menu: ContextMenuController
@@ -627,6 +641,7 @@ export default function PlexBrowse() { )} +
{isFetchingNextPage && (

{t("plex.loading")}

diff --git a/frontend/src/components/PlexCollectionView.tsx b/frontend/src/components/PlexCollectionView.tsx index 74efe5a8..5feef76d 100644 --- a/frontend/src/components/PlexCollectionView.tsx +++ b/frontend/src/components/PlexCollectionView.tsx @@ -11,6 +11,8 @@ import { showEpisodeKeys } from "../lib/plexQueue"; import { useConfirm } from "./ConfirmProvider"; import { BackBtn, ActionBtn, DetailHero } from "../lib/plexDetailUi"; import { PlexPosterCard } from "./plexCards"; +import { PlexPinMenu } from "./ComfortPinMenu"; +import { useContextMenu } from "./ui/useContextMenu"; import PlexLibraryPicker from "./PlexLibraryPicker"; import { toggleWatchState } from "../lib/plexWatch"; import { LoadingState, StateMessage } from "./ui/QueryState"; @@ -47,6 +49,8 @@ export default function PlexCollectionView({ const { t } = useTranslation(); const qc = useQueryClient(); const confirm = useConfirm(); + // One comfort-pin menu for the member grid (see ui/ContextMenu). + const pinMenu = useContextMenu(); const isAdmin = qc.getQueryData<{ role?: string }>(qk.me())?.role === "admin"; const q = useQuery({ queryKey: qk.plexCollections("detail", collectionId), @@ -348,6 +352,7 @@ export default function PlexCollectionView({ onOpen={() => onOpenCard(c)} onInfo={() => onInfoCard(c)} onToggleWatched={toggleWatched} + menuProps={pinMenu.triggerProps} /> {canEdit && (
); } diff --git a/frontend/src/components/PlexDiscover.tsx b/frontend/src/components/PlexDiscover.tsx index 858c126b..c7414ef5 100644 --- a/frontend/src/components/PlexDiscover.tsx +++ b/frontend/src/components/PlexDiscover.tsx @@ -10,6 +10,8 @@ import { toggleWatchState } from "../lib/plexWatch"; import { usePlex } from "./PlexProvider"; import { Carousel } from "./ui/Carousel"; import { PlexPosterCard } from "./plexCards"; +import { PlexPinMenu } from "./ComfortPinMenu"; +import { useContextMenu } from "./ui/useContextMenu"; import { LoadingState, StateMessage } from "./ui/QueryState"; // The Plex "Discover" landing (E5 S4): a stack of horizontal Carousel rails served from ONE @@ -40,6 +42,8 @@ function CardRail({ onToggleWatched: (c: PlexCard) => void; action?: ReactNode; // optional header control (e.g. the Surprise-me reshuffle button) }) { + // One comfort-pin menu per rail (see ui/ContextMenu), not one per poster. + const pinMenu = useContextMenu(); const heading = action != null ? ( @@ -59,10 +63,12 @@ function CardRail({ onOpen={() => onOpenCard(c)} onInfo={() => onInfoCard(c)} onToggleWatched={onToggleWatched} + menuProps={pinMenu.triggerProps} /> ))} + ); } diff --git a/frontend/src/components/PlexProvider.tsx b/frontend/src/components/PlexProvider.tsx index 53d46024..cc68890d 100644 --- a/frontend/src/components/PlexProvider.tsx +++ b/frontend/src/components/PlexProvider.tsx @@ -28,9 +28,22 @@ import { useNavigation } from "./NavigationProvider"; // search term is deliberately its OWN state, NOT the feed's `filters.q`, so a Plex search never leaks // into (and is never greeted by a stale value from) the feed. -// A "jump straight to this title" hand-off from the header typeahead: PlexBrowse opens a movie's -// info page (like the card's ⓘ) or a show's drill-down (like clicking a show card) by type. -export type PlexJumpTarget = { id: string; type: "movie" | "show" }; +// A "jump straight to this title" hand-off: PlexBrowse opens a movie's info page (like the card's ⓘ), +// a show's drill-down (like clicking a show card), or — for E24's Home hero, whose whole promise is a +// single Play button — the PLAYER for a movie/episode rating_key. `play` exists because the player +// lives inside this module with its lazy hls.js and its provider tree; mounting a second one on the +// landing page to avoid one hand-off would be the expensive way round. +export type PlexJumpTarget = { + id: string; + type: "movie" | "show" | "play"; + /** The jump ARRIVED WITH a page change — another module set this and sent the user here in one + * gesture (the Home surface plays a Plex title). The opened sub-view then REPLACES the page entry + * that hand-off just pushed, instead of stacking on top of it, so Back returns to where the user + * actually came from rather than to a module root they never visited. Omitted for a jump made + * from INSIDE the Plex module (the header typeahead), where the entry being replaced would be a + * sub-view the user really was looking at. */ + replace?: boolean; +}; type PlexState = { scope: string; // movie | show | both (unified cross-library scope) diff --git a/frontend/src/components/SettingsPanel.tsx b/frontend/src/components/SettingsPanel.tsx index e473d7b8..e32c270c 100644 --- a/frontend/src/components/SettingsPanel.tsx +++ b/frontend/src/components/SettingsPanel.tsx @@ -16,6 +16,7 @@ import { type SaveState } from "./ui/DraftSaveBar"; import { useConfirm } from "./ConfirmProvider"; import { StateMessage } from "./ui/QueryState"; import { useWizard } from "./WizardProvider"; +import { START_ON_HOME } from "./navigationInit"; import { usePrefs } from "./PrefsProvider"; import Tabs from "./ui/Tabs"; import { useMe } from "../lib/useMe"; @@ -129,6 +130,11 @@ function Appearance({ prefs }: { prefs: PrefsController }) { // Controlled by App's prefs draft: each change applies locally for preview but is only // persisted on an explicit Save (handled by the panel's Save/Discard bar). const { theme, setTheme, perf, setPerf, hints, setHints } = prefs; + // The landing page (E24). NOT a server preference like the rest of this tab: `loadInitialPage` + // has to answer before /api/me lands, so the value must be readable synchronously from + // localStorage — a landing module decided after the first paint would be a visible jump. The two + // stored values come from navigationInit, which is what reads them back at boot. + const [startOnHome, setStartOnHome] = useAccountPersistedState(LS.startOnHome, START_ON_HOME.on); return ( <> @@ -214,6 +220,19 @@ function Appearance({ prefs }: { prefs: PrefsController }) { +
+ + setStartOnHome(v ? START_ON_HOME.on : START_ON_HOME.off)} + /> + +
+
void; onToggleSave: (id: string, saved: boolean) => void; onOpen?: (v: Video, startAt?: number | null) => void; + /** Right-click / long-press props for the LIST's one context menu (E24 S3 — the comfort pin). + * A FUNCTION, not the already-built props object: `useContextMenu.triggerProps` is stable across + * renders while its return value is a fresh object every call, and a fresh object prop would + * blow past this component's memo for every card in the virtualized feed on every parent + * render. The card calls it with the video it already has. */ + menuProps?: (v: Video) => Record; }) { const { t, i18n } = useTranslation(); const { openChannel } = useNavigationActions(); @@ -521,6 +528,7 @@ function VideoCard({
{/* Actions lead: this is where the eye already is (the title is the thing you read), so @@ -583,6 +591,7 @@ function VideoCard({
`, so it is + // the same component all the way down. + const pinMenu = useContextMenu
))} @@ -259,6 +267,7 @@ export function PlexPersonView({ )}
)} + ); } diff --git a/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx b/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx index 6313e4b3..81cf801e 100644 --- a/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx +++ b/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx @@ -1,8 +1,10 @@ // @vitest-environment jsdom // // ONE test, ONE file, on purpose. This is the close→reclaim→run ordering contract of the palette -// (see closeThenNavigate), and it is the only assertion in the suite that drives a real -// useBackToClose entry through a real programmatic history.back(). +// (see closeThenNavigate), and it ASSERTS on a real useBackToClose entry driven through a real +// programmatic history.back() — which is why it cannot share module state with anything. (It is not +// the only place in the suite where an unstubbed back() fires; history.burial.dom.test.ts is the +// other file that asserts on one, and any test rendering a real Modal can let one go.) // // It cannot live in CommandPalette.dom.test.tsx. jsdom gives a test FILE a single session history, // and history.ts's overlay bookkeeping is module-global, so the sibling tests there leave an diff --git a/frontend/src/components/ui/DataTable.tsx b/frontend/src/components/ui/DataTable.tsx index b8271eba..81d1efa5 100644 --- a/frontend/src/components/ui/DataTable.tsx +++ b/frontend/src/components/ui/DataTable.tsx @@ -50,6 +50,7 @@ export default function DataTable({ persistKey, emptyText, rowClassName, + rowProps, controlsPosition = "bottom", controlsLeading, controlsInBand = false, @@ -70,6 +71,12 @@ export default function DataTable({ persistKey?: string; emptyText?: string; rowClassName?: (row: T) => string; + // Extra DOM props per row — the escape hatch a caller needs to hang an INPUT PATH on a row it does + // not own the markup of. Today's only consumer is `useContextMenu().triggerProps` (right-click / + // long-press / Shift+F10), and both row shapes get it, so the wide table and the narrow card + // answer the same gesture. Deliberately not a bag of named callbacks: the table stays domain-free + // and never learns what a menu is. + rowProps?: (row: T) => Record; controlsPosition?: "top" | "bottom" | "both"; controlsLeading?: ReactNode; // Render the controls row (controlsLeading + pager) into the shell's fixed toolbar band instead @@ -253,6 +260,7 @@ export default function DataTable({ {paged.map((row) => ( ({ {paged.map((row) => (
{columns diff --git a/frontend/src/components/ui/KeyboardHelp.dom.test.tsx b/frontend/src/components/ui/KeyboardHelp.dom.test.tsx index cf99270c..7949ff89 100644 --- a/frontend/src/components/ui/KeyboardHelp.dom.test.tsx +++ b/frontend/src/components/ui/KeyboardHelp.dom.test.tsx @@ -27,9 +27,9 @@ afterEach(() => { closeKeyboardHelp(); }); -/** App's job: it binds the `global` scope (h / Mod+K) once for the whole tree. */ +/** App's job: it binds the `global` scope (h / Mod+K / Shift+W) once for the whole tree. */ function Host({ children }: { children?: React.ReactNode }) { - useGlobalKeymap(); + useGlobalKeymap(true, () => {}); return ( <> {children} @@ -83,13 +83,13 @@ describe("KeyboardHelp", () => { const rows = descriptions(); // A plexPlayer row first, the app-wide rows last — the reading order the reader wants. expect(rows[0]).toBe("keys.playPause"); - expect(rows.slice(-2)).toEqual(["keys.palette", "keys.help"]); + expect(rows.slice(-3)).toEqual(["keys.palette", "keys.help", "keys.home"]); }); it("lists only the app-wide keys when no surface is registered", () => { render(); pressH(); - expect(descriptions()).toEqual(["keys.palette", "keys.help"]); + expect(descriptions()).toEqual(["keys.palette", "keys.help", "keys.home"]); }); // The context menu is NOT app-wide: it has its own scope, registered by `useContextMenu`, so the diff --git a/frontend/src/components/ui/QueryState.tsx b/frontend/src/components/ui/QueryState.tsx index 2aa85523..2b18abb2 100644 --- a/frontend/src/components/ui/QueryState.tsx +++ b/frontend/src/components/ui/QueryState.tsx @@ -29,17 +29,24 @@ export function StateMessage({ children, onRetry, tone = "muted", + compact = false, className = "", }: { children: ReactNode; onRetry?: () => void; tone?: "muted" | "error"; + /** A rung down in vertical rhythm, for a message that sits INSIDE a section that already has its + * own heading and padding (a rail's empty state) rather than standing alone on a page. Here + * rather than at the call site because the only two consumers so far had to reach for `!py-6` to + * beat this component's own `py-10` — the first `!important` overrides in the frontend, and the + * copyable idiom for the next surface that wants a compact empty state. */ + compact?: boolean; className?: string; }) { const { t } = useTranslation(); return (
{ + fetchMock.mockReset().mockResolvedValue({ + ok: true, + status: 200, + headers: new Headers({ "content-type": "application/json" }), + json: async () => ({}), + text: async () => "{}", + }); + vi.stubGlobal("fetch", fetchMock); +}); +afterEach(() => vi.unstubAllGlobals()); + +// Indexed, not `.at(-1)`: the e2e tsconfig's lib target predates it, and the gate typechecks three +// projects. +const lastUrl = (): string => { + const calls = fetchMock.mock.calls; + return String(calls[calls.length - 1]?.[0]); +}; + +describe("discoverApi query strings", () => { + it("sends the press count as `start` — the tier rotation lives on the wire", async () => { + await discoverApi.discoverPick("medium", 42, [], 3); + expect(lastUrl()).toContain("start=3"); + expect(lastUrl()).toContain("budget=medium"); + expect(lastUrl()).toContain("seed=42"); + }); + + it("sends the press count even on the first press, so `start` is never implied", async () => { + await discoverApi.discoverPick("any", 7); + expect(lastUrl()).toContain("start=0"); + }); + + it("encodes the exclusion list, whose keys carry a colon", async () => { + await discoverApi.discoverPick("any", 1, ["youtube:abc", "plex:99"], 2); + expect(lastUrl()).toContain(`exclude=${encodeURIComponent("youtube:abc,plex:99")}`); + }); + + it("omits `exclude` entirely when there is nothing to exclude", async () => { + await discoverApi.discoverPick("any", 1, [], 0); + expect(lastUrl()).not.toContain("exclude="); + }); +}); diff --git a/frontend/src/lib/api/discover.ts b/frontend/src/lib/api/discover.ts new file mode 100644 index 00000000..d3052278 --- /dev/null +++ b/frontend/src/lib/api/discover.ts @@ -0,0 +1,101 @@ +// The Home surface ("Mit nézzek most?", E24): one hero suggestion, a cross-source resume rail, and +// the comfort shelf. Types mirror `backend/app/discover/` — the card carries the NATIVE feed / Plex +// payload under `video` / `plex`, so the page renders through the existing VideoCard / +// PlexPosterCard instead of a third card component. +import { req } from "./core"; +import type { Video } from "./feed"; +import type { PlexCard } from "./plex"; + +// How much time the user says they have. The deterministic replacement for a mood question: the band +// also decides the source (ten minutes is a video, an evening is a film). Bands live server-side +// (`app/discover/budget.py`) — these are just the names on the wire. +export const DISCOVER_BUDGETS = ["short", "medium", "long", "any"] as const; +export type DiscoverBudget = (typeof DISCOVER_BUDGETS)[number]; + +// Where the hero came from. Travels with the card so the page can label it honestly instead of +// inferring "where you left off" over something the user has never opened. +export type HeroTier = "resume" | "comfort" | "surprise"; + +export type DiscoverKind = "video" | "movie" | "show" | "episode" | "channel" | "playlist"; + +// What can go on the comfort shelf. `plex_item` covers a movie or a single episode; a show is pinned +// as `plex_show` because "put that show on" resolves to whichever episode is next. +export const PIN_KINDS = ["video", "channel", "plex_item", "plex_show", "playlist"] as const; +export type PinKind = (typeof PIN_KINDS)[number]; + +export interface DiscoverCard { + source: "youtube" | "plex"; + kind: DiscoverKind; + id: string; + title: string; + subtitle: string | null; + thumb: string | null; + // The ARTWORK's shape, not the module's: a Plex episode ships a landscape still exactly like a + // YouTube video, while movies and shows ship posters. Lets a rail size its tiles without + // branching on `source`. + aspect: "wide" | "poster"; + duration_seconds: number | null; + position_seconds: number; + remaining_seconds: number | null; + status: string; + // Exactly one is set (both null for a channel / playlist tile, which are not themselves playable). + video: Video | null; + plex: PlexCard | null; + // Shelf entries only. + pin_id?: number; + /** What actually plays when the tile is clicked: a show's next episode, a channel's newest + * unwatched video, or a copy of the tile itself when it plays directly. Null when the pin has + * nothing to offer right now (a fully-watched channel, an empty playlist). */ + next?: DiscoverCard | null; + unwatched_count?: number; + item_count?: number; +} + +export interface DiscoverHero { + tier: HeroTier; + card: DiscoverCard; +} + +export interface DiscoverPayload { + budget: DiscoverBudget; + hero: DiscoverHero | null; + continue_watching: DiscoverCard[]; + comfort: DiscoverCard[]; +} + +export const discoverApi = { + discover: (budget: DiscoverBudget, seed: number): Promise => + req(`/api/discover?budget=${budget}&seed=${seed}`), + // The "Mást kérek" reroll: a new hero alone, so pressing it does not re-fetch the rails under the + // user's cursor. + // `exclude` is the rolling list of `:` keys already shown. Without it a reroll only + // moves the surprise tier, so a user with something half-finished gets the same hero back. + // `press` is which press this is; it rotates the tier the server answers from, so consecutive + // presses offer a different KIND of answer instead of walking down the resume rail. + discoverPick: ( + budget: DiscoverBudget, + seed: number, + exclude: string[] = [], + press = 0, + ): Promise<{ budget: DiscoverBudget; hero: DiscoverHero | null }> => + req( + `/api/discover/pick?budget=${budget}&seed=${seed}&start=${press}` + + (exclude.length ? `&exclude=${encodeURIComponent(exclude.join(","))}` : ""), + ), + comfortPins: (): Promise<{ items: DiscoverCard[] }> => req("/api/discover/pins"), + addComfortPin: ( + kind: PinKind, + ref: string, + ): Promise<{ ok: boolean; id: number; created: boolean }> => + req("/api/discover/pins", { method: "POST", body: JSON.stringify({ kind, ref }) }), + removeComfortPin: (pinId: number): Promise<{ deleted: number }> => + req(`/api/discover/pins/${pinId}`, { method: "DELETE" }), + reorderComfortPins: (ids: number[]): Promise<{ ok: boolean }> => + req("/api/discover/pins/reorder", { method: "POST", body: JSON.stringify({ ids }) }), +}; + +/** A card's identity across sources — mirrors `app/discover/cards.py::key`. The reroll's exclusion + * list and the "is this tile the hero?" check both speak it, so the two cannot drift. */ +export function discoverCardKey(card: Pick): string { + return `${card.source}:${card.id}`; +} diff --git a/frontend/src/lib/api/facade.test.ts b/frontend/src/lib/api/facade.test.ts index 4ac8b5e5..433109e8 100644 --- a/frontend/src/lib/api/facade.test.ts +++ b/frontend/src/lib/api/facade.test.ts @@ -20,6 +20,7 @@ const SLICE_MODULES = [ ["authApi", () => import("./auth")], ["setupApi", () => import("./setup")], ["feedApi", () => import("./feed")], + ["discoverApi", () => import("./discover")], ["channelsApi", () => import("./channels")], ["syncApi", () => import("./sync")], ["playlistsApi", () => import("./playlists")], diff --git a/frontend/src/lib/globalKeymap.dom.test.tsx b/frontend/src/lib/globalKeymap.dom.test.tsx index 2df7ec95..023f242c 100644 --- a/frontend/src/lib/globalKeymap.dom.test.tsx +++ b/frontend/src/lib/globalKeymap.dom.test.tsx @@ -30,8 +30,16 @@ afterEach(() => { }); /** Renders the live stores so an assertion reads what a real consumer would render. */ -function Host({ enabled = true, onState }: { enabled?: boolean; onState: (s: string) => void }) { - useGlobalKeymap(enabled); +function Host({ + enabled = true, + onState, + onHome = () => {}, +}: { + enabled?: boolean; + onState: (s: string) => void; + onHome?: () => void; +}) { + useGlobalKeymap(enabled, onHome); const help = useKeyboardHelpOpen(); const palette = usePaletteOpen(); useEffect(() => { @@ -111,6 +119,45 @@ describe("useGlobalKeymap", () => { expect(state).toBe("none"); }); + // E24. The chord is the point: the Plex player binds bare letters and `w` sits on top of `s`, so a + // bare `w` would yank a viewer out of the film on a blind reach for the subtitle key. + it("jumps Home on Shift+W and ignores a bare w", () => { + const onHome = vi.fn(); + render( {}} onHome={onHome} />); + key({ key: "w" }); + expect(onHome).not.toHaveBeenCalled(); + key({ key: "w", shiftKey: true }); + expect(onHome).toHaveBeenCalledTimes(1); + }); + + it("leaves Shift+W to the text field it was typed into", () => { + const onHome = vi.fn(); + render( {}} onHome={onHome} />); + const field = document.createElement("input"); + document.body.appendChild(field); + field.focus(); + act(() => { + field.dispatchEvent( + new KeyboardEvent("keydown", { key: "w", shiftKey: true, bubbles: true, cancelable: true }), + ); + }); + expect(onHome).not.toHaveBeenCalled(); + field.remove(); + }); + + // The handler is App's `() => setPage("home")` — a fresh closure every render. It is held in a ref + // rather than in the effect's deps, so the listener is bound once; this pins that the ref is the + // CURRENT one and not the closure captured at mount. + it("calls the latest handler after a re-render, without rebinding", () => { + const first = vi.fn(); + const second = vi.fn(); + const { rerender } = render( {}} onHome={first} />); + rerender( {}} onHome={second} />); + key({ key: "w", shiftKey: true }); + expect(first).not.toHaveBeenCalled(); + expect(second).toHaveBeenCalledTimes(1); + }); + // Bound above the branch that mounts the overlays, the keys were swallowed on the loading/Welcome // screens and the flipped store then sprang the overlay open by itself once the shell mounted. it("binds nothing, and clears a stale store, while disabled", () => { diff --git a/frontend/src/lib/history.burial.dom.test.ts b/frontend/src/lib/history.burial.dom.test.ts new file mode 100644 index 00000000..8b951b0c --- /dev/null +++ b/frontend/src/lib/history.burial.dom.test.ts @@ -0,0 +1,43 @@ +// @vitest-environment jsdom +// +// ONE test, ONE file, for the same reason CommandPalette.ordering.dom.test.tsx has its own: it +// ASSERTS on a reconcile issuing a REAL, unstubbed programmatic history.back(), and history.ts's +// overlay bookkeeping is module-global. history.dom.test.ts keeps its shared state clean by never +// leaving a real back() in flight — its layers close by a genuine Back, a stubbed back(), or a +// consumed burial mark — and this test would break that. +import { describe, it, expect, vi } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { navigateOverOverlays, useBackToClose } from "./history"; + +const settle = () => + act(async () => { + await Promise.resolve(); + }); + +describe("navigateOverOverlays · burying a navigation that never happened", () => { + // The round-2 defect. `navigateOverOverlays` marked every live overlay entry buried BEFORE it knew + // whether the navigation pushed anything, and `setPage` early-returns when the requested page is + // already current. Home is the default landing page, so Shift+W pressed while already on Home with + // the player open buried the player's entry under a push that never happened: its unmount then + // consumed the mark instead of reclaiming the entry with history.back(), leaving an orphan. The + // user's next browser Back popped onto an identical `sfPage` with an empty overlay stack — a press + // that changed nothing on screen, one more per play → Shift+W → close cycle. + it("leaves the overlay's entry reclaimable when the navigation reports it pushed nothing", async () => { + const overlay = renderHook(() => useBackToClose(() => {})); + await settle(); + const back = vi.spyOn(window.history, "back"); + try { + // What `setPage` returns from its early return: asked for the page we are already on. + const navigate = vi.fn(() => false); + act(() => navigateOverOverlays(navigate)); + expect(navigate).toHaveBeenCalledTimes(1); + + overlay.unmount(); + await settle(); + // Nothing was pushed over it, so the entry is still ours — reclaimed, not silently orphaned. + expect(back).toHaveBeenCalledTimes(1); + } finally { + back.mockRestore(); + } + }); +}); diff --git a/frontend/src/lib/history.dom.test.ts b/frontend/src/lib/history.dom.test.ts index 38cb7e8b..4cf0c926 100644 --- a/frontend/src/lib/history.dom.test.ts +++ b/frontend/src/lib/history.dom.test.ts @@ -7,11 +7,15 @@ // A GENUINE user Back is simulated by dispatching `popstate` directly: the real-Back branch in // history.ts does NOT call history.back() (it only decrements its own historyDepth to match the // entry the browser already dropped), so a raw popstate reproduces it deterministically without -// depending on jsdom's async back() semantics. Each test closes its layers via genuine Backs, so -// no reconcile-driven history.back() runs and the shared module state stays clean between tests. +// depending on jsdom's async back() semantics. The invariant that keeps the module-global +// bookkeeping clean between tests here is NOT "only genuine Backs" — two closeThenNavigate cases +// deliberately drive a reconcile into history.back() — it is that no REAL back() is ever left in +// flight: a layer is closed by a genuine Back, by a STUBBED back() that lands its own popstate, or +// by an unmount whose burial mark is consumed. A test that needs an unstubbed one belongs in its own +// file (see history.burial.dom.test.ts). import { describe, it, expect, vi, afterEach } from "vitest"; import { renderHook, act } from "@testing-library/react"; -import { closeThenNavigate, useBackToClose, useBackToExit } from "./history"; +import { closeThenNavigate, navigateOverOverlays, useBackToClose, useBackToExit } from "./history"; // Let the mount effect's coalesced reconcile microtask settle. const settle = () => @@ -96,7 +100,11 @@ describe("closeThenNavigate", () => { const order: string[] = []; closeThenNavigate( () => order.push("close"), - () => order.push("navigate"), + // Braced: `navigate` reports whether it PUSHED (see `buryUnder`), so a bare `push` returning + // an array length is a type error rather than an accidental "yes". + () => { + order.push("navigate"); + }, ); expect(order).toEqual(["close"]); @@ -148,6 +156,37 @@ describe("closeThenNavigate", () => { } }); + // The mirror of the case above, and the OTHER door into `buryUnder` — round 3 found that only + // `navigateOverOverlays` was pinned, so this call site could be reverted to the old unconditional + // burial with the whole suite green. Reachable through the palette: `run: () => setPage(page)` is + // a bare arrow, so `setPage`'s `false` really does arrive here when the chosen module is the one + // already on screen. Nothing was pushed, so the layer underneath is still ours to reclaim. + it("leaves the layer below reclaimable when the command navigated nowhere", async () => { + const back = vi.spyOn(window.history, "back").mockImplementation(() => { + window.dispatchEvent(new PopStateEvent("popstate", { state: window.history.state })); + }); + try { + const player = renderHook(() => useBackToClose(() => {})); + await settle(); + const palette = renderHook(() => useBackToClose(() => {})); + await settle(); + + const navigate = vi.fn(() => false); // setPage's early return: already on that page + await act(async () => { + closeThenNavigate(() => palette.unmount(), navigate); + await Promise.resolve(); + }); + expect(navigate).toHaveBeenCalledTimes(1); + expect(back).toHaveBeenCalledTimes(1); // the palette's own entry + + player.unmount(); + await settle(); + expect(back).toHaveBeenCalledTimes(2); // and the player's, because nothing was pushed over it + } finally { + back.mockRestore(); + } + }); + // Without this the command is not late, it never happens: an overlay closed in the same tick it // opened never pushed an entry, so no reclaim popstate is ever coming. it("navigates anyway when no reclaim popstate ever arrives", () => { @@ -163,3 +202,27 @@ describe("closeThenNavigate", () => { } }); }); + +describe("navigateOverOverlays", () => { + // The global-shortcut half of the same trap `closeThenNavigate` solves. Shift+W does not know an + // overlay is open and cannot close it — it just navigates, and the page change unmounts the player + // (which lives inside the module column). That unmount's reconcile then reclaimed the entry the + // navigation had only just pushed, so the player closed and the page snapped straight back. + it("a navigation made from outside an open overlay survives that overlay's unmount", async () => { + const overlay = renderHook(() => useBackToClose(() => {})); + await settle(); + const back = vi.spyOn(window.history, "back"); + try { + const navigate = vi.fn(() => window.history.pushState({ sfPage: "home" }, "")); + act(() => navigateOverOverlays(navigate)); + expect(navigate).toHaveBeenCalledTimes(1); + + overlay.unmount(); // what the page change does to the player + await settle(); + // The mark is consumed instead: no back(), so the entry `navigate` pushed is still current. + expect(back).not.toHaveBeenCalled(); + } finally { + back.mockRestore(); + } + }); +}); diff --git a/frontend/src/lib/history.subview.dom.test.ts b/frontend/src/lib/history.subview.dom.test.ts index 75603f4f..1705a92a 100644 --- a/frontend/src/lib/history.subview.dom.test.ts +++ b/frontend/src/lib/history.subview.dom.test.ts @@ -76,6 +76,42 @@ describe("useHistorySubview open", () => { expect(window.history.state?._sub).toBe("root"); expect(window.location.search).toBe(""); // ?v dropped }); + + // E24: a cross-module hand-off (Home plays a Plex title) arrives on an entry the page navigation + // just pushed. Opening the sub-view on TOP of it stranded the user on a module root they never + // visited — browser-Back from the player landed on the Plex Discover page instead of Home + // (measured 2026-09-06). `replace` makes the sub-view BE that entry. + it("replace rewrites the current entry instead of stacking a new one", () => { + const before = window.history.length; + const { result } = renderHook(() => useHistorySubview("root", url)); + act(() => result.current.open("a", { replace: true })); + expect(result.current.view).toBe("a"); + expect(window.history.state?._sub).toBe("a"); + expect(window.location.search).toBe("?v=a"); + expect(window.history.length).toBe(before); // no new entry to Back through + }); + + it("replace is idempotent — two opens of the same view are still one entry", () => { + // Not a nicety: under StrictMode the jumpTo effect body runs TWICE before its `setJumpTo(null)` + // re-renders, and two pushes of the identical sub-view left a duplicate whose Back visibly did + // nothing. + const before = window.history.length; + const { result } = renderHook(() => useHistorySubview("root", url)); + act(() => { + result.current.open("a", { replace: true }); + result.current.open("a", { replace: true }); + }); + expect(window.history.length).toBe(before); + }); + + it("keeps the entry's own sfScroll id on a replace", () => { + // It is the SAME entry, so it keeps its scroll identity; PageScroller keys offsets per + // (entry, view), and the view key changed, so the sub-view still opens at the top. + setUrl("/", { sfScroll: 7 }); + const { result } = renderHook(() => useHistorySubview("root", url)); + act(() => result.current.open("a", { replace: true })); + expect(window.history.state?.sfScroll).toBe(7); + }); }); describe("useHistorySubview popstate", () => { diff --git a/frontend/src/lib/history.ts b/frontend/src/lib/history.ts index 1f33789b..f9b03510 100644 --- a/frontend/src/lib/history.ts +++ b/frontend/src/lib/history.ts @@ -81,7 +81,10 @@ export function useHistorySubview( url?: SubviewUrl, ): { view: T; - open: (next: T) => void; + /** `replace` rewrites the CURRENT entry instead of stacking a new one — for a sub-view that is the + * continuation of the navigation that just arrived here, not a step the user took inside the + * module. See the `jumpTo` hand-off in PlexBrowse. */ + open: (next: T, opts?: { replace?: boolean }) => void; back: () => void; } { // Resolve the current entry's sub-view: history.state._sub for an in-app entry, else decode the @@ -110,9 +113,17 @@ export function useHistorySubview( }, []); return { view, - open: (next: T) => { + open: (next: T, opts?: { replace?: boolean }) => { setView(next); const href = url ? urlWithParams({ [url.key]: url.encode(next) }) : ""; + if (opts?.replace) { + // Same ENTRY, so it keeps its sfScroll id: PageScroller keys offsets per (entry, view), and + // the view key has changed, so the sub-view still opens at the top without minting an id + // that would orphan the one this entry already carries. + window.history.replaceState({ ...window.history.state, _sub: next }, "", href); + notifyNav(); + return; + } // A fresh sfScroll so the opened sub-view is its own scroll entry (starts at the top, and Back // restores the view we came from) — then notify PageScroller, which this push doesn't re-render. window.history.pushState( @@ -153,6 +164,37 @@ let buriedEntries = 0; let reconcileScheduled = false; let listenerInstalled = false; +/** Run a navigation and mark the live overlay entries buried under it — but ONLY if it really + * pushed an entry. A navigation that reports `false` pushed nothing, so its overlays are still on + * top and must stay reclaimable: `setPage` early-returns when the requested page is already + * current, and Home is the default landing page, so "Shift+W while already on Home with the player + * open" navigated nowhere yet still buried the player's entry. The player's unmount then consumed + * the mark instead of issuing history.back(), orphaning its browser entry — and the next Back + * popped onto an identical `sfPage` with an empty overlay stack, i.e. a press that did nothing + * (round 2). One rule for both entry points below — but read the next paragraph before trusting it + * at the `closeThenNavigate` one, where most callers cannot report at all. + * + * It fails OPEN: only a literal `false` skips the mark, so a `navigate` that reports nothing is + * assumed to have pushed. That is what the code did unconditionally before, so nothing regressed by + * adding the rule — it just does not yet reach everywhere. Today only `navigateOverOverlays` + * (App's Shift+W → `setPage`) is fully covered. Of `closeThenNavigate`'s callers, only a palette + * row written as a bare `() => setPage(page)` forwards the boolean: `CommandItem.run` is typed + * `() => void`, so the block-bodied and non-navigating rows report nothing — and neither does + * `openChannel`, which has its OWN no-push branch (it `replaceState`s when the entry already + * carries `_chan`) and is reached both from a palette row and from PlayerModal's channel-name jump. + * Each of those still marks a live sibling overlay's entry buried under a push that never happened, + * costing one dead browser-Back press when that overlay is closed by its own control. + * + * Closing it is three changes, not one — widen `CommandItem.run` to `() => boolean | void`, return + * the verdict from the block-bodied rows, and give `openChannel` the boolean `setPage` now has — so + * it is deferred to the project backlog rather than bolted on here. + * + * Marking AFTER the call is safe: `navigate()` is synchronous and only `reconcileHistory` (a + * microtask) and the popstate listener ever move `historyDepth`. */ +function buryUnder(navigate: () => boolean | void): void { + if (navigate() !== false) buriedEntries = historyDepth; +} + // Push/pop history entries until their count matches the live overlay count. Runs once per tick. function reconcileHistory(): void { reconcileScheduled = false; @@ -210,7 +252,7 @@ function ensureOverlayListener(): void { * * Only call this from INSIDE an overlay that uses `useBackToClose`: the popstate we wait for is * that overlay's own entry being reclaimed, and without one it never arrives. */ -export function closeThenNavigate(close: () => void, navigate: () => void): void { +export function closeThenNavigate(close: () => void, navigate: () => boolean | void): void { let done = false; // Armed BEFORE the close, so it is a plain const and there is no declaration order to reason about. // Degrades honestly if no popstate is coming at all — an overlay closed in the same tick it opened @@ -222,7 +264,6 @@ export function closeThenNavigate(close: () => void, navigate: () => void): void done = true; window.removeEventListener("popstate", onPop); clearTimeout(fallback); - navigate(); // Every overlay entry still counted is now BURIED under the entry the navigation just pushed, so // a later unmount must NOT reclaim it with history.back() — that pops the navigation instead and // silently bounces the user back where they started (a palette opened over the player: closing @@ -230,8 +271,10 @@ export function closeThenNavigate(close: () => void, navigate: () => void): void // reconcile ate the new page entry). MARK them rather than zeroing the depth: round 1 set // `historyDepth = 0` outright, which also desynced the counter from overlays the navigation does // NOT unmount — About/ReleaseNotes live outside the page column — so one Back both navigated and - // closed them (round 2). - buriedEntries = historyDepth; + // closed them (round 2). `buryUnder` skips the mark when the command reports it navigated + // nowhere — which here is only the palette's bare `() => setPage(page)` module rows; see the + // fail-open limit documented on `buryUnder` for the rows that cannot report. + buryUnder(navigate); }; const onPop = () => { // The same reconcile can issue MORE than one back(); a pushState made between them would be @@ -243,6 +286,30 @@ export function closeThenNavigate(close: () => void, navigate: () => void): void close(); } +/** Navigate from OUTSIDE any overlay — a global keyboard shortcut — when one may be open. + * + * `closeThenNavigate` is for a control that lives INSIDE the overlay and can close it, and it waits + * for that overlay's own entry to come back. A global key has neither: it does not know which + * overlay is open, or whether one is at all. It just navigates, and the page change unmounts + * whatever the module column was holding — whose reconcile then reclaims the entry the navigation + * had just pushed. Shift+W over an open player closed the player and snapped back to the page it + * started on, which reads as a dead shortcut. + * + * Same fix as `closeThenNavigate`, one step shorter: bury the live overlay entries FIRST, so a + * later unmount consumes the marks instead of issuing `history.back()`. There is no popstate to + * wait for, because nothing is being closed on purpose. An overlay the navigation does NOT unmount + * (the palette and the cheat sheet live above the page column) keeps its entry buried too — the + * same trade `closeThenNavigate` already makes, and the reason closing the palette afterwards no + * longer pops the page out from under the user. + * + * `navigate` reports whether it actually pushed — see `buryUnder`, which is why `setPage` returns a + * boolean rather than this guessing from `history.length` (a `back()` followed by a `pushState` + * leaves it unchanged) or from the entry's `sfScroll` id (`openChannel` pushes without minting + * one). */ +export function navigateOverOverlays(navigate: () => boolean | void): void { + buryUnder(navigate); +} + /** While the calling component is mounted, Back (or the browser/mouse back button) closes it via * `onClose` instead of navigating away. Mount the component only while the overlay is open. */ export function useBackToClose(onClose: () => void): void { diff --git a/frontend/src/lib/homeIntent.ts b/frontend/src/lib/homeIntent.ts new file mode 100644 index 00000000..f58d173f --- /dev/null +++ b/frontend/src/lib/homeIntent.ts @@ -0,0 +1,30 @@ +import { createStore } from "./store"; + +// A one-shot "…and surprise me straight away" request for the Home surface (E24), handed over by +// callers that cannot reach its state — today the command palette's "Meglepetés". +// +// A TOKEN that waits, not an event that fires: Home is a lazy module and is normally NOT mounted +// when the request is made (the palette navigates to it in the same click), so an event would be +// shouted into an empty room. Home takes the token on arrival, which clears it, so walking back to +// Home an hour later does not replay the press — a stale reroll would look like the page deciding +// on its own that the answer you came back for was wrong. +const surprise = createStore(0); + +/** Ask Home to reroll its hero as soon as it is on screen. Pair it with a `setPage("home")`. */ +export const requestSurprise = (): void => surprise.set((n) => n + 1); + +/** The pending token (0 = nothing pending) — the render subscription that wakes Home up. Home only. */ +export const useSurpriseRequest = (): number => surprise.use(); + +/** Take the pending request, clearing it; false when there was nothing to take. + * + * Compare-and-clear rather than a plain read + `consume()`, because a reroll is NOT idempotent — it + * spends a `/discover/pick` and steps the tier rotation — and React 18's StrictMode invokes a mount + * effect twice with the same render snapshot. Measured in the browser (2026-09-07): the palette's + * "Meglepetés" from another module arrived at Home as `start=2`, two picks deep, before the page had + * painted once. Checking the LIVE store is what makes the second pass a no-op. */ +export const takeSurprise = (): boolean => { + if (!surprise.get()) return false; + surprise.set(0); + return true; +}; diff --git a/frontend/src/lib/keymap.test.ts b/frontend/src/lib/keymap.test.ts index 44b37a84..681a3c25 100644 --- a/frontend/src/lib/keymap.test.ts +++ b/frontend/src/lib/keymap.test.ts @@ -146,6 +146,7 @@ describe("dispatch", () => { const handlers = { help: () => void calls.push("help"), palette: () => void calls.push("palette"), + home: () => void calls.push("home"), }; expect(dispatch(press({ key: "F10", shiftKey: true }), "global", handlers)).toBe(false); expect(dispatch(press({ key: "ContextMenu" }), "global", handlers)).toBe(false); diff --git a/frontend/src/lib/keymap.ts b/frontend/src/lib/keymap.ts index d907e692..ee47c000 100644 --- a/frontend/src/lib/keymap.ts +++ b/frontend/src/lib/keymap.ts @@ -1,4 +1,4 @@ -import { useEffect } from "react"; +import { useEffect, useRef } from "react"; import { createStore } from "./store"; // ── The app's ONE keyboard map ──────────────────────────────────────────────────────────────── @@ -56,6 +56,10 @@ export const SHORTCUTS = [ // (E10 review round 1). Its handler preventDefaults, so nothing leaks to the browser either way. { id: "palette", scope: "global", keys: ["Mod+k"], labelKey: "keys.palette", allowInInput: true }, { id: "help", scope: "global", keys: ["h"], labelKey: "keys.help" }, + // Shift+W and not a bare `w` (E24): the Plex player binds bare letters (a s d f h i k m) and `w` + // sits directly above `s`, so a blind reach for the subtitle key would yank you out of the film + // and onto the landing page. A chord makes it the deliberate "I'm stuck — answer the question". + { id: "home", scope: "global", keys: ["Shift+w"], labelKey: "keys.home" }, // --- the row context menu (wherever useContextMenu is wired) --------------------------------- // Its OWN scope, not `global`: only the surfaces that actually call `useContextMenu` register it, // so the sheet stops advertising "Open the context menu" on the feed, Plex, Downloads and Settings @@ -281,11 +285,16 @@ export const usePaletteOpen = (): boolean => paletteOpen.use(); * key belonging to the other, and `dispatch`'s exhaustiveness would stop meaning anything. App * calls this; the overlays are pure renderers of their store. * - * `enabled` must track where those overlays are actually MOUNTED. Bound unconditionally it swallowed - * `h` and Mod+K on the loading/Welcome/setup screens — the browser shortcut was preventDefault'ed, - * nothing appeared, and the flipped module-level store then popped the overlay open by itself the - * moment the signed-in shell mounted (E10 review round 1). */ -export function useGlobalKeymap(enabled = true): void { + * `enabled` must track where those overlays (and the pages Shift+W jumps to) are actually MOUNTED. + * Bound unconditionally it swallowed `h` and Mod+K on the loading/Welcome/setup screens — the + * browser shortcut was preventDefault'ed, nothing appeared, and the flipped module-level store then + * popped the overlay open by itself the moment the signed-in shell mounted (E10 review round 1). */ +export function useGlobalKeymap(enabled: boolean, onHome: () => void): void { + // The Home jump is the one global key whose action lives OUTSIDE this file — navigation is a + // provider and `lib/` may not import a component — so App hands it in. Kept in a ref so a fresh + // closure each render doesn't rebind the window listener (and can't leave a stale `setPage`). + const homeRef = useRef(onHome); + homeRef.current = onHome; useEffect(() => { if (!enabled) { // Leaving a stale `true` behind would spring the overlay open on the next mount. @@ -294,10 +303,11 @@ export function useGlobalKeymap(enabled = true): void { return; } const onKey = (e: KeyboardEvent) => { - // Both global keys TOGGLE an overlay, and each overlay mount/unmount costs a real history - // entry (useBackToClose). On auto-repeat that is ~30 push/back pairs a second, which Chrome - // starts dropping while our own depth bookkeeping keeps counting them — a desynced stack whose - // next browser Back walks out of the app. Holding a key is not a second press. + // Two of the three global keys TOGGLE an overlay, and each overlay mount/unmount costs a real + // history entry (useBackToClose); the third (Shift+W) pushes a page entry of its own. Either + // way, on auto-repeat that is ~30 push/back pairs a second, which Chrome starts dropping while + // our own depth bookkeeping keeps counting them — a desynced stack whose next browser Back + // walks out of the app. Holding a key is not a second press. if (e.repeat) return; dispatch(e, "global", { help: (ev) => { @@ -309,6 +319,10 @@ export function useGlobalKeymap(enabled = true): void { ev.preventDefault(); paletteOpen.set((v) => !v); }, + home: (ev) => { + ev.preventDefault(); + homeRef.current(); + }, }); }; window.addEventListener("keydown", onKey); diff --git a/frontend/src/lib/modules.ts b/frontend/src/lib/modules.ts index 5268fa4b..d1a119cb 100644 --- a/frontend/src/lib/modules.ts +++ b/frontend/src/lib/modules.ts @@ -12,6 +12,7 @@ import { SlidersHorizontal, Tv, Users, + Youtube, type LucideIcon, } from "lucide-react"; import type { Me } from "./api"; @@ -46,7 +47,11 @@ const notDemo = (me: Me) => !me.is_demo; // Ordered in nav-rail order — this array IS the sequence the rail and the stepper walk. const MODULES: readonly ModuleDef[] = [ - { id: "feed", labelKey: "header.account.feed", icon: Home }, + // Home leads: it is the landing (see navigationInit.loadInitialPage), and the module order IS the + // nav order and the header stepper's sequence. The feed takes the YouTube mark its own toolbar + // already uses, so the two are apart by shape and not just by position. + { id: "home", labelKey: "home.navLabel", titleKey: "home.title", icon: Home }, + { id: "feed", labelKey: "header.account.feed", icon: Youtube }, { id: "channels", labelKey: "header.account.channels", diff --git a/frontend/src/lib/pageMeta.test.ts b/frontend/src/lib/pageMeta.test.ts index b9152d12..eea55f63 100644 --- a/frontend/src/lib/pageMeta.test.ts +++ b/frontend/src/lib/pageMeta.test.ts @@ -6,6 +6,7 @@ import type { Page } from "./urlState"; // browser-tab title keys the old hand-written switch produced — pin every page so a future labelKey // rename (or a dropped titleKey) can't silently change a tab title. const EXPECTED: Record = { + home: "home.title", feed: "header.account.feed", channels: "header.channelManager", playlists: "header.account.playlists", diff --git a/frontend/src/lib/queryKeys.ts b/frontend/src/lib/queryKeys.ts index d72a7ea1..6bb7bbae 100644 --- a/frontend/src/lib/queryKeys.ts +++ b/frontend/src/lib/queryKeys.ts @@ -100,6 +100,15 @@ export const qk = { // array, a "union"/"group"/"contains" discriminator), so they take a variadic pass-through: // the value this factory centralizes is the ROOT string, and TanStack keys are `unknown[]` // anyway. Single-id plex keys stay precisely typed. + // The Home surface (E24). `discover()` with no args is the prefix every mutation invalidates — + // pinning something changes the shelf AND can change the hero, so the two are never invalidated + // separately. The reroll has its own key so pressing "Mást kérek" does not refetch the rails — + // but it sits UNDER the same root, because a sibling root ("discover-pick") is not matched by + // that prefix and the rerolled hero silently outlived every invalidation on the page: play it, + // finish it, and it was still the answer on screen with a zeroed progress bar. + discover: (...rest: unknown[]) => ["discover", ...rest], + discoverPick: (...rest: unknown[]) => ["discover", "pick", ...rest], + comfortPins: () => ["comfort-pins"] as const, plex: () => ["plex"] as const, plexWatchLink: () => ["plex-watch-link"] as const, plexLibrary: (...rest: unknown[]) => ["plex-library", ...rest], diff --git a/frontend/src/lib/releaseNotes.ts b/frontend/src/lib/releaseNotes.ts index 2be7e330..d424e8d9 100644 --- a/frontend/src/lib/releaseNotes.ts +++ b/frontend/src/lib/releaseNotes.ts @@ -14,6 +14,26 @@ export interface ReleaseEntry { } export const RELEASE_NOTES: ReleaseEntry[] = [ + { + version: "0.83.0", + date: "2026-09-09", + summary: + "Siftlode now opens on a page that answers one question: what should I watch right now. You say how much time you have \u2014 ten minutes, half an hour, a whole evening \u2014 and it gives you one suggestion, from YouTube or from Plex, with a button that picks up where you left off. Under it sits a shelf of the things you always come back to, which you fill yourself.", + features: [ + "A new Home page, and it is where Siftlode starts. It asks how much time you have \u2014 10 minutes, 30 minutes, an evening, or it does not matter \u2014 and answers with ONE suggestion rather than a wall of thumbnails. The question is on top, the answer directly under it. If you would rather land on the feed as before, Settings \u2192 Appearance \u2192 Startup turns it off.", + "The time you have picks the source, not just the length. Ten minutes gets you a short video; an evening is allowed to offer a film. It matches on the time you have LEFT in something, so a series episode you are four minutes from finishing counts as a four-minute answer.", + "The suggestion is one of three things, and it says which: something you left half-finished, something off your comfort shelf, or a surprise. Continue takes you back to the second you stopped. Something else rerolls it \u2014 there is no separate surprise-me page, the reroll is it.", + "A comfort shelf you fill yourself, for the media you put on without thinking. It takes a whole channel, a whole show, a film, a single video or a playlist \u2014 because comfort viewing is put on TheVR, not play this one episode. One click resolves it: a show gives you the next unwatched episode, a channel its newest unwatched video. The shelf can be reordered into the order you want.", + "Pin from wherever you are: the same \u22ef menu \u2014 or a right-click \u2014 on a feed video, a channel row, a playlist, a Plex poster or a Plex row now carries a Comfort shelf entry. Feed video cards answer a right-click for the first time.", + "Continue watching, on both sides at once. The rail merges what you left half-watched on YouTube with what you left half-watched on Plex, most recently touched first \u2014 the YouTube half of that did not exist before.", + "Shift+W jumps to Home from anywhere, including out of a player, and it is in the keyboard sheet. The jump-to box (Ctrl+K) gained a Surprise me row, and typing what should I watch finds Home itself.", + ], + chores: [ + "E24, four sprints. S1 built a discover read-model as its own backend package \u2014 the unified card shape maps the EXISTING Plex and feed serializers rather than adding a base serializer, the rails reuse the feed query layer and the Plex rollup, and the tiered hero rule is one shared function that returns the tier it chose, so the UI prints an honest kicker instead of guessing. The shelf is a typed comfort_pins table with a one-of check constraint and a real foreign key per kind, not a polymorphic string reference: a deleted Plex item must not leave a dead pin on the shelf. S2 built the page on the existing carousel and the two existing card components rather than a third. S3 wired pinning through the context-menu input path E10 shipped, which absorbed that epic's parked video-card item. S4 added the ways in.", + "The closing review ran four rounds, 46 \u2192 23 \u2192 19 \u2192 15 \u2192 14, each round reviewing the previous round's own fix diff, with three adversarial verifiers per finding in rounds one to three. Four of round one's findings died at 2-of-3 and are recorded so they are not raised again. What the eight-lane gate could not have caught: the resume rail built its own query and dropped the per-user channel blocklist, so a video from a channel you had just banned could become the landing page's suggestion; the shelf numbered a new pin by row COUNT while nothing renumbers, so after any non-tail unpin a pin landed mid-shelf and could collide; the Home tile and the show page disagreed about which episode comes next, one taking the first in-progress episode and the other the last; and Shift+W was dead whenever anything was open, because the page change unmounted the player and its cleanup reclaimed the history entry the navigation had just pushed. Rounds three and four stopped finding defects in the epic and started finding them in the review's own fix commits \u2014 a test named after a branch it never reached, comments claiming more than the code does \u2014 which is the signal to stop.", + "Two process notes worth keeping. A commit in this epic fixed a bug that does not exist: its message and two comments claimed a sort key raises when two rows both lack a timestamp, which tuple comparison never reaches \u2014 three verifiers executed it independently, and the rationale is now corrected in place rather than quietly deleted. And three of the regression tests written during the fix rounds could not go red for the defect they named until they were rewritten, which is why every test added in a fix round on this epic was mutation-checked.", + ], + }, { version: "0.82.0", date: "2026-09-06", diff --git a/frontend/src/lib/storage.ts b/frontend/src/lib/storage.ts index 6610d308..c260fb03 100644 --- a/frontend/src/lib/storage.ts +++ b/frontend/src/lib/storage.ts @@ -15,6 +15,11 @@ export const LS = { lang: "siftlode.lang", filters: "siftlode.filters", page: "siftlode.page", + // E24: start every session on the Home surface rather than on the module you happened to leave + // open. Per-account, "0" = opt out and go back to remembering the last module. It has to be + // localStorage rather than a server pref because `loadInitialPage` runs before /api/me lands — + // a landing page decided after the first paint would be a visible jump. + startOnHome: "siftlode.startOnHome", perfMode: "siftlode.perfMode", // The feed's view mode (see lib/feedView.ts) — a cache of the server pref, so a reload of an // established tab paints your view straight away. A brand-new tab still starts on the default diff --git a/frontend/src/lib/urlState.ts b/frontend/src/lib/urlState.ts index bfa4470c..871899be 100644 --- a/frontend/src/lib/urlState.ts +++ b/frontend/src/lib/urlState.ts @@ -119,6 +119,7 @@ export function hasFilterParams(params: URLSearchParams): boolean { // derive from it, so adding a page is a one-line change (no parallel allowlists to keep in // sync). "feed" is the default/fallback. const PAGES = [ + "home", "feed", "channels", "stats",