Deep-review round 3 of bug hunt 2026-09.
- get_or_404 / owned_or_404 answer a structured {code: "not_found", reason: <what>}
(errors.notFound.* in HU+EN, generic fallback). The structured-404 fix of round 2 had gone into the
ad-hoc `_own_playlist` (CLAUDE.md's "Not" pattern) while the shared helpers stayed string-only, so
views, tags, pins, Plex playlists, invites and users still 404'd in English. `_own_playlist` now
goes through owned_or_404; admin's hand-rolled user lookups use get_or_404.
- A move into a list that is gone names the TARGET (reason "move_target") — "this playlist doesn't
exist any more" was read as the list on screen, which still exists.
- Every refusal in the playlists router is a {code: "playlist", reason} (playlists.errors.* HU+EN)
instead of a bare English string shown verbatim in the Hungarian UI.
- `req` / `uploadReq` mark the errors they already put on screen (the error dialog, the connection
notice, the 401 sign-out), and notifyActionError skips those. The watched toggles, the saved views
and a playlist delete toasted a fault the dialog was already showing; Playlists' two hand-written
status filters now go through the same check.
198 lines
7.8 KiB
Python
198 lines
7.8 KiB
Python
"""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 `<source>:<id>` 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, what="pin")
|
|
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}
|