Full-epic /deep-review (11 finders + 12 adversarial verifiers) surfaced no HIGH; every
mechanism finding calibrated to LOW on verification. Fix all 13:
Correctness / reliability:
- Combobox: Enter now commits the VISUALLY highlighted row (highlightIndex ?? activeIndex),
not the bare keyboard activeIndex — so arrow-then-hover-then-Enter no longer toggles a
different option than the one painted, and hover-with-no-arrow is no longer a silent no-op.
- parse_person: dedup the known_for pool by (id, media_type), not the bare id — TMDB movie
and tv id spaces are independent, so a same-id movie+show pair is two distinct credits
(matches the collision-safe key the rest of the module already uses).
- person-enrich: a per-person in-flight + 60s cooldown guard (people._should_schedule_enrich)
dedups concurrent opens and rate-limits re-triggering while TMDB is degraded (a transient
TmdbError leaves the watermark NULL), so bursts can't pile 8s tasks onto the shared threadpool.
- PlexInfo compact()/money(): format with i18n.language (keeping currency USD) instead of a
hardcoded en/en-US, matching the app's Intl convention.
- delete the dead `plex.filter.add` i18n key (both locales) — the +Add button that used it is gone.
Reuse:
- _continue_watching reuses rollup.agg_subquery(include_resumed=True) + query.apply_show_state_filter
instead of a third hand-inlined copy of the per-show rollup + in_progress predicate, so the
Discover rail can't drift from the grid's definition.
Test coverage (the gaps a regression would ship through):
- resolve_library_guids scope-exclusion (a credit in a non-scoped/disabled library is dropped)
+ the []-vs-None distinction; parse_person cross-media-id retention; enrich_person transient
(NULL watermark) and 404 (stamp) branches; _enrich_person_bg body (fresh session close/rollback,
None-guard) + the cooldown dedup; continue_watching fully-watched-show exclusion; Combobox
Enter-commits-highlighted + async/controlled-query mode; PopoverItem aria-checked contract;
RatingsRow missing-IMDb + imdb==audience branches.
Backend lane 413→420, FE vitest 351→357; all gates green. No migration.
Address the deep-review's deferred backend items so E5 is review-complete:
- person-enrich OFF the request path (ABUSE finding): person_detail no longer makes
the TMDB call inline (even bounded to 8s it sat on the serve thread, un-throttled,
demo-reachable — it violated the module's "no live TMDB call at serve time"
invariant). It now schedules a FastAPI BackgroundTask (people._enrich_person_bg,
fresh session, best-effort) after the response; the first open serves the row as-is
and the bio lands by the next open. The inline try/rollback recovery is gone with it.
- reuse: a shared tmdb.resolve_library_guids owns the collision-safe guid→library
bucketing (movie vs tv ids are independent sequences) that resolve_recs and
people._resolve_known_for each hand-duplicated; a new rollup.item_state_map dedups
the "PlexState-by-item_id" fetch across the person + discover paths.
- test gaps: enrich_batch transient TmdbError leaves the watermark NULL to retry;
guid_to_tmdb_id resolves a tvdb source + tv media_type via /find's tv_results;
_continue_watching includes a NULL-status started movie and excludes a hidden one;
/api/search/youtube returns 422 (not 5xx) when ingest fails (route-level, B-core-1).
Backend lane 403 → 413, ruff clean. No migration; serve contracts unchanged.
Three verified deep-review findings (the 4th, the filter combobox a11y gap, is
deferred to a dedicated session on the combobox architecture):
- SEC (medium): TmdbClient sent the api_key as a URL query param, so an HTTP
error's httpx exception string embedded the full URL incl. the key, which
enrich_batch logged. _get now raises status+path only (never str(e)/the URL)
and severs the secret-bearing cause with `from None`. Regression test asserts
neither the status nor the network error path contains the key.
- STATE (low): person_detail's lazy-enrich except now db.rollback()s, so a
failed enrich_person commit can't poison the session and 500 the subsequent
SELECTs (SQLAlchemy 2.x deactivates the tx on a commit-time flush failure and
does not auto-reset it). Test locks that the page renders despite a poisoned
enrich.
- ABUSE (low): the request-path lazy person enrich now caps its TMDB client at
an 8s timeout (was the 30s default) so a slow TMDB can't tie up a worker;
TmdbClient gained a `timeout` param (batch keeps 30s). The fuller fix (move
person enrichment off the request path) is logged for later.
Follow-ups from the epic-end /code-review of the E5 branch:
- tmdb.resolve_recs + people._resolve_known_for: match recommendations/
credits to the library bucketed by media_type. TMDB movie ids and tv ids
are independent sequences, so a bare `tmdb:<id>` match could deep-link a
movie rec to an unrelated show (or vice-versa) on a numeric collision.
Regression test added (test_resolve_recs_matches_by_media_type_not_bare_id).
- routes/search: wrap the _ingest_candidates call so a DB/enrich failure
degrades to a 422 with a message instead of a bare 500 hidden behind the
FE's generic "search failed" (extends the B-core-1 rationale); rollback on
failure. Stamp enriched_at with its own timestamp taken after the enrich,
not the stub-insert placeholder.
- routes/plex/discover: key serialized cards back by rating_key (globally
unique in Plex) instead of id(obj); fix the stale rows 3-tuple annotation.
The person page now leads with a TMDB bio, lazily fetched the first time it's
opened (off the S2-linked `tmdb_person_id`) and then served from the row:
- Migration 0072: ADD-only nullable person columns on plex_people — biography,
birthday, deathday, place_of_birth, known_for_department, also_known_as,
known_for ([{tmdb_id, media_type, title}]), tmdb_enriched_at.
- tmdb.py: TmdbClient.fetch_person (/person/{id}+combined_credits), parse_person
(bio + a popularity-ranked, deduped known_for), apply_person, enrich_person
(lazy one-shot, key+id guarded, commits, 404 stamps the watermark).
- people.py person_detail: loads the full person row, lazily enriches (best-effort
— never fails the page), serves the bio fields + resolves known_for to library
rows by guid (in-library only, popularity order).
- FE PlexPersonView: bio/birth-facts/aka hero + a "Known for" Carousel rail above
the existing Movies/Shows filmography; PlexPersonDetail type + i18n (en+hu).
Tests: test_plex_person_bio.py (6: parse / lazy-enrich guards / serve) + extended
PlexPersonView.dom.test.tsx + the unknown-name contract in test_plex_person.py.
Browser-E2E'd on Jim Carrey (live TMDB enrich → bio 1937 chars, born 1962-01-17
Newmarket, Known-for resolved). Gate: vitest 337, backend lane 402.
A dedicated Plex "Discover" landing — a stack of horizontal Carousel rails
served from ONE consolidated read (serve-from-DB, no live Plex/TMDB call):
- GET /api/plex/discover → continue_watching (in-progress movies + shows, most
recently progressed) · recently_added · genres (top-N tiles) ·
because_you_watched (recs of the most-recent watched, TMDB-enriched movie,
resolved to library rows by guid via serve_block's resolver).
- GET /api/plex/surprise → a fresh random, scope-bounded sample for the ↻ rail.
Frontend: PlexDiscover.tsx (rails reuse the S1 Carousel + S3 PlexPosterCard;
colored genre tiles → tap to filter the grid; ↻ reshuffles the surprise rail
alone). Discover is now the root sub-view; the flat grid becomes the bookmarkable
"Browse all" secondary view (?v=browse) reached by a header search/filter (a
browse-intent flip) or the new Discover⟷Browse toggle.
resolve_recs is promoted from _resolve_recs (now cross-module). Tests:
test_plex_discover.py (rail shape/order, continue-watching, because-you-watched
+ null, surprise bounds, scope) and PlexDiscover.dom.test.tsx; plexSubUrl
round-trip updated for discover-root/grid-browse. Backend lane 393, vitest 332.
A live smoke against the real TMDB API showed two crew problems the mocked
tests missed: (1) a TV show's creator/showrunner lives in the payload's
top-level `created_by`, not the crew list, so it was dropped entirely; and
(2) TMDB's crew order is arbitrary — a TV show's per-episode Directors (dozens)
flooded the key-crew list and buried the show-level roles.
Fix: split the accepted crew jobs by media type — movies key on
Director/Writer/…; TV keys on Creator (from created_by) + the show-level
Executive Producer/Writer/Producer/Composer, deliberately excluding the
per-episode Director/Editor/DP noise. Stored crew is sorted by display
priority and capped, so the lead roles come first.
Verified live: Breaking Bad -> [Gilligan (Creator), Johnson (EP), … , Writer];
The Godfather -> [Coppola (Director), Puzo (Screenplay), …]. test_plex_tmdb
green (9).
Matches each mirrored Plex movie/show to TMDB by its stored canonical guid,
fetches once (append_to_response), and PERSISTS the enrichment onto the row —
served from the DB, never a live TMDB call at serve time. Additive: the
Plex-sourced imdb_rating/rating are untouched; tmdb_rating is the distinct
TMDb community score (ratings scope = TMDb + Plex's IMDb/audience, no OMDb/RT).
- config.py: tmdb_api_key + api_base/language/interval/batch defaults.
- sysconfig SPECS: tmdb_api_key (secret, Fernet) + tmdb_language, auto-rendered
in the admin Config page; read like the Plex token.
- migration 0071 (ADD-only nullable, reversible): plex_items + plex_shows gain
tmdb_id / tmdb_enriched_at (watermark) / tmdb_rating / tmdb_vote_count /
budget / revenue / keywords / crew / studios / production_countries /
recommendations / backdrops; plex_people gains tmdb_person_id (the S2
foundation for S6 person bios). No watermark reset needed.
- app/plex/tmdb.py: TmdbClient (mirrors PlexClient; throttled; 404->None,
transient->TmdbError), parse_tmdb / apply_tmdb_detail (one-parser rule),
guid_to_tmdb_id (tmdb: direct, imdb:/tvdb: via /find), enrich_batch (movies
then shows, transient error leaves the watermark NULL to retry, definitive
no-match still stamps so it isn't re-fetched), _link_people (upserts
tmdb_person_id by name), serve_block (recommendations resolved to library
rows by guid), run_enrich (no-op without a key).
- scheduler: tmdb_enrich nightly job (default 120 min, no YouTube quota),
runtime-tunable + "run now" from the Scheduler dashboard.
- detail endpoints emit `tmdb` (movies + shows) so it's curl-verifiable now;
the FE render lands in S5.
Note: _link_people inserts plex_people rows for TMDB cast/crew; unreferenced
crew (composers etc., not in cast_names/directors) get pruned by the orphan
sweep — acceptable, the person page is cast-driven and cast rows survive.
Gate: ruff + py_compile green; backend test lane FULL suite 387 passed
(incl. migration drift downgrade->re-upgrade + test_plex_tmdb 9 tests).
Frontend unchanged since S1. Not merged (epic branch, stacks on S1).
peter