diff --git a/backend/app/discover/hero.py b/backend/app/discover/hero.py index c124c45c..df7a3320 100644 --- a/backend/app/discover/hero.py +++ b/backend/app/discover/hero.py @@ -10,6 +10,10 @@ Three tiers, tried in order: 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 @@ -22,30 +26,18 @@ answer. from sqlalchemy.orm import Session -from app.discover import cards, pins, queries, rails +from app.discover import cards, pins, rails from app.discover.budget import fits -from app.models import PlexShow, User -from app.plex.lookups import by_rating_key +from app.models import User HERO_TIERS = ("resume", "comfort", "surprise") -# How deep the surprise tier draws before giving up. The client's exclusion window is 8 (see -# `Home.tsx`), so a draw of 12 can always step past a full window of already-shown picks. +# 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 _playable(db: Session, uid: int, card: dict) -> dict | None: - """A hero has to have a Play button, so a Plex SHOW is resolved to the episode that would start. - Everything else already plays itself. None when the show has no episodes mirrored yet.""" - if not (card.get("source") == "plex" and card.get("kind") == "show"): - return card - 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 pick( db: Session, user: User, @@ -55,6 +47,7 @@ def pick( 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. @@ -66,25 +59,46 @@ def pick( 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.""" + 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() - for card in rails.continue_watching(db, user) if resume is None else resume: - playable = _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 {"tier": "resume", "card": playable} - 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 {"tier": "comfort", "card": card} + 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 - # 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 {"tier": "surprise", "card": card} + 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/queries.py b/backend/app/discover/queries.py index c1136cc2..8e9b2341 100644 --- a/backend/app/discover/queries.py +++ b/backend/app/discover/queries.py @@ -275,12 +275,21 @@ def plex_surprise(db: Session, uid: int, budget: str, seed: int, limit: int) -> 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 — in broadcast order, the same `(season, episode)` 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.""" - rows = db.execute( + 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") @@ -288,20 +297,26 @@ def next_episode(db: Session, uid: int, show_id: int) -> dict | None: PlexItem.season_number.asc().nullsfirst(), PlexItem.episode_number.asc().nullsfirst(), ) - ).all() - if not rows: + .limit(1) + ) + # In progress. `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)) + ).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))) + ).first() + ) + # Fully watched show: start it again from the top. + pick = pick or db.execute(base).first() + if pick is None: return None - def _started(st: PlexState | None) -> bool: - return bool(st and st.position_seconds > 0 and st.status not in ("watched", "hidden")) - - def _unseen(st: PlexState | None) -> bool: - return st is None or st.status not in ("watched", "hidden") - - pick = next((r for r in rows if _started(r[1])), None) - pick = pick or next((r for r in rows if _unseen(r[1])), None) - pick = pick or rows[0] - ep, st = pick show = db.get(PlexShow, show_id) return leaf_card( diff --git a/backend/app/discover/rails.py b/backend/app/discover/rails.py index eac51240..4a622898 100644 --- a/backend/app/discover/rails.py +++ b/backend/app/discover/rails.py @@ -13,7 +13,8 @@ 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 User +from app.models import PlexShow, User +from app.plex.lookups import by_rating_key # Sort sentinel for a row whose timestamp is missing. A plain `(ts is not None, ts)` key looks like it # handles NULLs and does not: two NULL rows compare equal on the flag and then compare `None < None`, @@ -27,18 +28,49 @@ def plex_available(db: Session) -> bool: 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 too, and it is what the user - recognises); resolving it to an episode is the hero's job, where something actually has to play.""" + 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) - return [card for _, card in rows[:limit]] + 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]: diff --git a/backend/app/routes/discover.py b/backend/app/routes/discover.py index b9cf4e92..fa516c62 100644 --- a/backend/app/routes/discover.py +++ b/backend/app/routes/discover.py @@ -70,6 +70,7 @@ 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: @@ -80,11 +81,17 @@ def get_pick( `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.""" + 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])), + "hero": hero_mod.pick( + db, user, budget, seed, exclude=set(list(skip)[:MAX_EXCLUDE]), start=start + ), } diff --git a/backend/tests/test_discover.py b/backend/tests/test_discover.py index 1b51d191..1c83efb2 100644 --- a/backend/tests/test_discover.py +++ b/backend/tests/test_discover.py @@ -388,6 +388,37 @@ def test_the_reroll_must_be_able_to_move_past_the_resume_tier(db): 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.""" @@ -491,6 +522,67 @@ def test_a_self_playing_pin_does_not_reference_itself(db): 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.""" + user, _ = _seed_yt(db) + _seed_plex(db, user) + _enable_plex(db) + for card in rails.continue_watching(db, user): + 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_prefers_the_one_in_progress(db): """In progress beats "next unwatched": resuming is cheaper than starting.""" user, _ = _seed_yt(db) diff --git a/frontend/src/components/Home.dom.test.tsx b/frontend/src/components/Home.dom.test.tsx index 1b2e5e1d..ac5b3ba7 100644 --- a/frontend/src/components/Home.dom.test.tsx +++ b/frontend/src/components/Home.dom.test.tsx @@ -51,9 +51,16 @@ vi.mock("../lib/api", () => ({ discoverCardKey: (c: { source: string; id: string }) => `${c.source}:${c.id}`, })); -// The page hands Plex playback off to the Plex module; neither provider is under test here. -vi.mock("./NavigationProvider", () => ({ useNavigationActions: () => ({ setPage: vi.fn() }) })); -vi.mock("./PlexProvider", () => ({ usePlexActions: () => ({ setJumpTo: vi.fn() }) })); +// The page hands Plex playback off to the Plex module. The provider itself is not under test, but +// WHAT is handed over is: the jump target decides both what plays and where Back goes. +const setPage = vi.fn(); +const setJumpTo = vi.fn(); +vi.mock("./NavigationProvider", () => ({ + useNavigationActions: () => ({ setPage: (p: string) => setPage(p) }), +})); +vi.mock("./PlexProvider", () => ({ + usePlexActions: () => ({ setJumpTo: (t: unknown) => setJumpTo(t) }), +})); import Home from "./Home"; @@ -93,6 +100,39 @@ function ytCard(id: string, title: string, extra: Record = {}) }; } +/** A Plex SHOW row of the resume rail, carrying the episode that would actually start — what the + * backend now attaches (`rails.continue_watching`). */ +const showCard = () => ({ + source: "plex", + kind: "show", + id: "show-1", + title: "A Show", + subtitle: null, + thumb: null, + aspect: "poster", + duration_seconds: null, + position_seconds: 0, + remaining_seconds: null, + status: "in_progress", + video: null, + plex: { id: "show-1", type: "show", title: "A Show", thumb: "" }, + next: { + source: "plex", + kind: "episode", + id: "ep-7", + title: "Ep 7", + subtitle: "A Show", + thumb: null, + aspect: "wide", + duration_seconds: 1500, + position_seconds: 0, + remaining_seconds: 1500, + status: "new", + video: null, + plex: { id: "ep-7", type: "episode", title: "Ep 7", thumb: "" }, + }, +}); + const payload = (heroId: string, tier = "resume") => ({ budget: "any", hero: { tier, card: ytCard(heroId, `Hero ${heroId}`) }, @@ -112,6 +152,8 @@ function mount() { beforeEach(() => { discover.mockReset(); discoverPick.mockReset(); + setPage.mockReset(); + setJumpTo.mockReset(); discover.mockResolvedValue(payload("v1")); }); afterEach(cleanup); @@ -160,7 +202,7 @@ describe("Home", () => { await screen.findByText("Hero v1"); fireEvent.click(screen.getByText("home.hero.reroll")); await waitFor(() => - expect(discoverPick).toHaveBeenCalledWith("any", expect.any(Number), ["youtube:v1"]), + expect(discoverPick).toHaveBeenCalledWith("any", expect.any(Number), ["youtube:v1"], 1), ); expect(await screen.findByText("Rerolled")).toBeTruthy(); }); @@ -181,4 +223,60 @@ describe("Home", () => { mount(); expect(await screen.findByText("home.comfort.empty")).toBeTruthy(); }); + + it("plays a half-watched SHOW's next episode instead of opening the show page", async () => { + // Reported 2026-09-06. A show tile that opens the show page hands back the very decision this + // surface exists to remove — six seasons to choose between. The rail row carries `next`, and the + // click follows it. + discover.mockResolvedValue({ + budget: "any", + hero: null, + continue_watching: [showCard()], + comfort: [], + }); + mount(); + fireEvent.click(await screen.findByText("A Show")); + expect(setJumpTo).toHaveBeenCalledWith(expect.objectContaining({ id: "ep-7", type: "play" })); + }); + + it("hands off with `replace` so Back returns HERE, not to the Plex module's root", async () => { + // The hand-off is a continuation of the navigation that just pushed the Plex page entry, not a + // second step on top of it — without this, Back landed on the Plex Discover page the user never + // visited (and, under StrictMode, did nothing at all the first press). + discover.mockResolvedValue({ + budget: "any", + hero: null, + continue_watching: [showCard()], + comfort: [], + }); + mount(); + fireEvent.click(await screen.findByText("A Show")); + expect(setJumpTo).toHaveBeenCalledWith(expect.objectContaining({ replace: true })); + expect(setPage).toHaveBeenCalledWith("plex"); + }); + + it("rotates the tier on each press so the button offers a different KIND of answer", async () => { + // Measured 2026-09-06: exclusion alone left it walking the resume rail and cycling nine cards, + // never once reaching the shelf. The press count travels to the server as the rotation. + discoverPick.mockResolvedValue({ + budget: "any", + hero: { tier: "comfort", card: ytCard("v9", "Rerolled") }, + }); + mount(); + await screen.findByText("Hero v1"); + fireEvent.click(screen.getByText("home.hero.reroll")); + await waitFor(() => + expect(discoverPick).toHaveBeenLastCalledWith("any", expect.any(Number), ["youtube:v1"], 1), + ); + await screen.findByText("Rerolled"); + fireEvent.click(screen.getByText("home.hero.reroll")); + await waitFor(() => + expect(discoverPick).toHaveBeenLastCalledWith( + "any", + expect.any(Number), + expect.any(Array), + 2, + ), + ); + }); }); diff --git a/frontend/src/components/Home.tsx b/frontend/src/components/Home.tsx index 9995e6bf..b2c7f6a7 100644 --- a/frontend/src/components/Home.tsx +++ b/frontend/src/components/Home.tsx @@ -61,9 +61,12 @@ const BUDGETS: readonly DiscoverBudget[] = ["short", "medium", "long", "any"]; // drift from it. const YT_SLOT = 200; -// How many already-offered cards the reroll remembers. Matched by the server's own draw depth -// (`hero.SURPRISE_DRAW`), which has to be able to step past a full window. -const SEEN_WINDOW = 8; +// How many already-offered cards the reroll remembers. Matched to the server's own cap +// (`routes/discover.MAX_EXCLUDE`) rather than to a smaller number of our own: at 8 the window rolled +// before the resume rail (20 rows) was exhausted, so the button cycled the same nine cards forever +// (measured, 2026-09-06). It is bounded at all only because the server will not scan an unbounded +// exclusion list. +const SEEN_WINDOW = 32; // The four shelf moves, in the order they appear in the menu. const SHELF_MOVES: readonly [ShelfMove, string, typeof ChevronLeft][] = [ @@ -337,6 +340,10 @@ export default function Home() { // 0 = "the hero the page loaded with". Any other value is a reroll, answered by its own query so // pressing the button does not refetch the rails under the user's cursor. const [seed, setSeed] = useState(0); + // Which press this is. It rotates the tier the server leads with, so "Mást kérek" offers a + // different KIND of answer each time (where you left off → your shelf → a surprise) rather than + // the next row of the same rail. Reset with the budget: a new question starts the rotation over. + const [press, setPress] = useState(0); // The cards the reroll has already offered this session, newest last. A ROLLING window rather than // the whole history: an unbounded list would eventually exclude everything and leave the button // with nothing to give, whereas a window of 8 can never exhaust a library and only repeats after @@ -353,8 +360,8 @@ export default function Home() { placeholderData: keepPreviousData, }); const pickQ = useQuery({ - queryKey: qk.discoverPick(budget, seed, seen), - queryFn: () => api.discoverPick(budget, seed, seen), + queryKey: qk.discoverPick(budget, seed, seen, press), + queryFn: () => api.discoverPick(budget, seed, seen, press), enabled: seed !== 0, staleTime: 0, // Hold the previous hero while the next one is in flight. Without this the reroll's new query key @@ -415,7 +422,14 @@ export default function Home() { return; } if (target.plex) { - setJumpTo({ id: target.plex.id, type: target.plex.type === "show" ? "show" : "play" }); + // `replace`: the Plex module's sub-view rewrites the page entry `setPage` is about to push, + // so browser-Back from the player returns HERE — not to the Plex Discover root, which this + // navigation passed through and the user never saw. + setJumpTo({ + id: target.plex.id, + type: target.plex.type === "show" ? "show" : "play", + replace: true, + }); setPage("plex"); } }, @@ -427,7 +441,11 @@ export default function Home() { const onInfo = useCallback( (card: DiscoverCard) => { if (!card.plex) return; - setJumpTo({ id: card.plex.id, type: card.plex.type === "show" ? "show" : "movie" }); + setJumpTo({ + id: card.plex.id, + type: card.plex.type === "show" ? "show" : "movie", + replace: true, + }); setPage("plex"); }, [setJumpTo, setPage], @@ -485,6 +503,7 @@ export default function Home() { // would look like a dead button). if (heroKey) setSeen((prev) => [...prev.filter((k) => k !== heroKey), heroKey].slice(-SEEN_WINDOW)); + setPress((n) => n + 1); setSeed((prev) => { let next = prev; while (next === prev || next === 0) next = Math.floor(Math.random() * 1_000_000) + 1; @@ -515,6 +534,7 @@ export default function Home() { setBudget(v); setSeed(0); // the payload for the new budget carries its own hero setSeen([]); // a new question deserves the whole library again + setPress(0); // …and the tier rotation starts from the page's own priority order }} options={BUDGETS.map((b) => ({ value: b, label: t(`home.budget.${b}`) }))} /> diff --git a/frontend/src/components/PlexBrowse.tsx b/frontend/src/components/PlexBrowse.tsx index 626fda8a..0be8d279 100644 --- a/frontend/src/components/PlexBrowse.tsx +++ b/frontend/src/components/PlexBrowse.tsx @@ -141,6 +141,11 @@ export default function PlexBrowse() { : jumpTo.type === "play" ? { kind: "player", id: jumpTo.id } : { kind: "info", id: jumpTo.id }, + // A cross-module hand-off continues the navigation that brought us here rather than adding + // to it. It also makes this effect idempotent, which matters more than it looks: under + // StrictMode the effect body runs twice before `setJumpTo(null)` re-renders, and two pushes + // of the SAME sub-view left a duplicate entry whose Back visibly did nothing. + { replace: jumpTo.replace }, ); setJumpTo(null); } diff --git a/frontend/src/components/PlexProvider.tsx b/frontend/src/components/PlexProvider.tsx index 43af0150..cc68890d 100644 --- a/frontend/src/components/PlexProvider.tsx +++ b/frontend/src/components/PlexProvider.tsx @@ -33,7 +33,17 @@ import { useNavigation } from "./NavigationProvider"; // 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" }; +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/lib/api/discover.dom.test.ts b/frontend/src/lib/api/discover.dom.test.ts new file mode 100644 index 00000000..960e2e82 --- /dev/null +++ b/frontend/src/lib/api/discover.dom.test.ts @@ -0,0 +1,52 @@ +// @vitest-environment jsdom +// +// The Home surface's slice builds its own query strings, and the Home DOM tests mock this module — +// so nothing else in the suite ever looks at the URL that actually goes out. That gap hid a live +// parameter once already: the reroll's `start` reached `api.discoverPick` and stopped there. +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { discoverApi } from "./discover"; + +const fetchMock = vi.fn(); + +beforeEach(() => { + 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 index dcbab493..d3052278 100644 --- a/frontend/src/lib/api/discover.ts +++ b/frontend/src/lib/api/discover.ts @@ -70,13 +70,16 @@ export const discoverApi = { // 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}` + + `/api/discover/pick?budget=${budget}&seed=${seed}&start=${press}` + (exclude.length ? `&exclude=${encodeURIComponent(exclude.join(","))}` : ""), ), comfortPins: (): Promise<{ items: DiscoverCard[] }> => req("/api/discover/pins"), 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..28d9f4f5 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(