Skip to content

feat(catalog): episode-level title cards (apply + display) - #240

Merged
randrini merged 3 commits into
mainfrom
fix/v1-episode-artwork
Oct 7, 2026
Merged

randrini merged 3 commits into
mainfrom
fix/v1-episode-artwork

Conversation

@randrini

@randrini randrini commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Closes #229
Validation tasks: none found covering episode artwork.

Episode title cards are a core part of the curated-artwork workflow, but the images API only supported show/season-level types - users could not apply complete per-episode sets.

Approach

Server: the episode still pipeline already existed end to end (still_path/thumbhash, resolver mapping, publish, serve); the gap was type recognition. Map "titlecard" to the existing still type (no new type invented, no migration): accepted on episode scope, 400 on any other scope with no download. Surface the episode still in the picker current view (additive still_url, v1 body unchanged). Web: episode detail hero selects source by URL then uses that source's thumbhash (never mixed), preferring the title card with series-backdrop and season-row fallbacks; no new components or loading paths. The v1 wrong-scope behavior change (silent poster store → 400) is documented as a bridge correctness fix; v1 response shapes unchanged.

Validation

  • API tests (request mapping, wrong-scope 400 matrix incl. v1 policy pins, v1/v2 omission pins) + DB-backed publication → detail/row readback (sibling/parent untouched)
  • Web: hero source-pairing cases + full ItemDetail + full web suite green; tsc/eslint/prettier clean
  • Contract additive-only; route inventory current; full CI green on the final head

Risks

The v2 response change is additive. The shared apply handler intentionally rejects non-episode titlecard requests that previously overwrote the scope's poster; documented request mappings and v1 response shapes remain unchanged. Apple/Android: no work required (existing image components, no new fields).

Checklist

  • I read and can explain the complete diff.
  • This pull request addresses one concern.

AI Disclosure

  • Harness: OpenCode
  • Tool(s): OpenCode (fixer, designer subagents)
  • Model(s): opencode-go/muse-spark-1.3-contributor
  • Involvement: AI-assisted

randrini added 2 commits October 7, 2026 13:44
Accept `type: "titlecard"` for an episode on
`POST /api/v2/admin/items/{id}/images/apply`. The name aliases the pipeline's
`still` in `metadata.ImageTypeFromString`, so an episode title card is cached
and published through the existing episode-still path with no schema change
(the `episodes` table already carries `still_path`). Because the resolver
aliases to `ImageStill`, `artworkTargetColumns` accepts it only for an episode
and refuses a movie, series, or season with `400 unsupported_image_type`
before any download, instead of silently storing it as a poster.

Surface the episode's current title card to the image picker: the shared
handler `current` view now carries `still_url`, marked `json:"-"` so the
frozen v1 body is unchanged, and apiv2 emits it as an additive `still_url`.

Contract artifacts are regenerated. The OpenAPI change is additive only:
an optional response property plus a description on the existing `type` field.
The regenerated web contract types are left for the UI lane.
The episode detail hero always painted the parent series' backdrop, so the
one piece of artwork an episode owns — its still — arrived on the detail
payload as `poster_url` and was never drawn. Prefer it for the hero
backdrop: a still is 16:9, the same slot the backdrop already occupies, so
the page keeps its layout and DetailHero's existing fade-in and thumbhash
placeholder. The generated thumbnail therefore stands in while the title
card loads, and is what shows when the episode has no title card at all.

The chain keeps the series backdrop in second place, so an episode with
nothing of its own renders exactly as it did before, and reaches the
season's episode row last. Episode rows name this artwork `still_url` —
with the series art substituted when the episode has none — which is the
only source left when the detail carries neither a still nor a backdrop (a
series with a poster but no backdrop). The thumbhash resolves in the same
order so the placeholder always belongs to the image it is replacing, and
the page glow is keyed to whichever thumbhash the hero shows, the way the
ebook and manga pages key theirs to their poster.

No admin picker for episode art: Edit Metadata still excludes the Images
section for episodes, and building one is out of scope here.

Regenerate the committed v2 web bindings so the additive `still_url` on
the image picker's `current` view and the widened `type` description are
typed; `make verify-apiv2-web-types` reports the pair current.

EpisodeContent tests cover all three sources of hero artwork.
@drondeseries

Copy link
Copy Markdown
Collaborator

Production review: REQUEST CHANGES

Backend reuse is sound and it's one coherent concern, but 3 mediums block merge. CI was still IN_PROGRESS at review time — full green on the final head required regardless.

1. Medium — hero can pair an image with another image's thumbhash

web/src/pages/ItemDetail/EpisodeContent.tsx:283-286: URL and thumbhash use independent || chains, so they can select different sources (e.g. episode with still URL but no still thumbhash shows the title card with the series backdrop's placeholder/ambient color; retained still thumbhash with no still URL rides along on the backdrop). DetailHero.tsx:127-148 paints that placeholder under the image, so it's visible.
Fix: choose the source by URL, then use that source's thumbhash (may be absent). Add tests: preferred URL with no hash, hash whose URL is absent.

2. Medium — new catalog docs describe fields episode detail doesn't carry

docs/catalog-api.md:476-480 (added by this PR) says episode rows and details carry still_url/still_thumbhash with series fallback. Episode detail actually serves poster_url = PresignImageURL(episode.StillPath, "still", …) + poster_thumbhash = episode.StillThumbhash plus separate series backdrop_url (internal/catalog/detail.go:3231-3232,3252-3253). Clients following the new docs read nonexistent fields — that's introduced here, not pre-existing drift, so don't defer it.
Fix: document row fields separately from detail fields, and backend row fallback separately from the web hero order. Docs-only, no new detail fields needed.

3. Medium — v1 "unchanged" claim needs reconciling

ImageTypeFromString is shared by both transports: "titlecard" used to fall through to ImagePoster, now it's ImageStill (internal/metadata/service.go:8475), and wrong-scope rejects before ApplyItemImage (admin_images.go:349-361, artwork_selection.go:319-341). So the same v1 movie/series/season apply request changes from poster-apply to 400 unsupported_image_type. json:"-" hides no response field but doesn't isolate that request-behavior change on a frozen surface.
Fix: explicitly rule this an allowed bridge correctness fix and document/test it as such, or normalize the alias at the v2 boundary preserving v1. Don't build version machinery before settling the policy.

4. Low — "apply stores still" test doesn't store

admin_images_titlecard_test.go:64-92 fails the image service before publication — checks conversion + season/episode args, but never stores, never asserts status/DB/serve. Useful preflight, overstated name/summary.
Fix: rename to request-mapping, and add/point to an integration test for successful publication → detail/row reads (incl. still replacement, sibling/parent untouched).

What's good

No new type/migration ("titlecard" → ImageStill, serialized "still"); wrong-scope rejects before remote work (movie/series/season tests assert zero service calls); existing transactional publication + GC reuse; ClassActingAdmin + acting-admin middleware intact; no new IDOR (targets from DB resolution, parameterized IDs); v2 still_url additive with OpenAPI/types/fixture; no new fetch/N+1; metadata events + immutable revision URLs cover refresh; jellycompat already projects stills (batch_loaders.go:472-480, mapping.go:793-794).

Before merge

  1. Fix 1-3, settle 🟠 Web: array-typed plugin config fields silently wiped to [] on save #4's naming/evidence.
  2. Full CI green on final head (was IN_PROGRESS).
  3. Live verify: apply → display → reapply with different bytes (path/hash/revision change, parent/sibling unchanged, open page refreshes, missing hash / failed image load). Note the || chain covers absent URLs, not HTTP load failure.
  4. Pin v1 omission + v2 unset omission in serialization tests (currently shared-unset + v2-set only).
  5. Record Apple/Android follow-up or explicit no-work rationale ([v1] Episode-level artwork (title cards) #229 mentions display parity); clarify the "generated thumbnail" fallback is existing still/series art + placeholder, not new frame generation.
  6. Add validation-task assessment + manual-impact disposition.

…ard policy

Address the PR #240 review.

Web: pick the episode hero artwork by URL first, then use that same source's
thumbhash even when it is absent. The independent `||` chains could show one
image behind another image's generated thumbnail. Tests cover a preferred still
with no hash and an orphaned hash whose URL is absent.

Docs: describe episode rows and episode details separately. Rows carry
still_url/still_thumbhash with a series fallback; details serve the still as
poster_url/poster_thumbhash and the series backdrop separately, and the web
hero order is documented apart from the backend row fallback.

Tests: pin the v1 bridge ruling that titlecard on a non-episode scope is now a
400 rather than a silent poster store — an allowed correctness fix, not a
contract change — alongside the unchanged mapping for every previously-valid
request. Rename the request-mapping test and add integration coverage that a
successful apply publishes to detail and row reads with the sibling and parent
untouched. Add the v1 still_url omission and the v2 unset-omission pins.
@randrini

randrini commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Review addressed and pushed (6ac0b5ed): hero now selects source by URL then uses that source's thumbhash (tests for hash-less preferred URL + orphaned hash); docs split row vs detail fields and backend vs web fallback orders; v1 titlecard-on-wrong-scope 400 documented + tested as bridge correctness fix with v1 omission pins; request-mapping test renamed with DB-backed publication→detail/row integration (sibling/parent untouched). Web suites + related Go tests green. Validation tasks: none found covering episode artwork. Apple/Android: no work required (existing components, no new fields). Full CI required before merge.

@drondeseries

Copy link
Copy Markdown
Collaborator

Review at 6ac0b5ed — APPROVE

All 4 prior findings verified fixed. No new code blockers. 8/8 CI green, MERGEABLE/CLEAN confirmed.

1. Hero mismatch → fixed. web/src/pages/ItemDetail/EpisodeContent.tsx:287-296 picks the source by URL first, then uses that same source's thumbhash (absent allowed, no fall-through to another hash). Order preserved: episode detail poster → series backdrop → episode row still. Tests EpisodeContent.test.tsx:508-550 cover preferred-still-with-no-hash + orphaned-hash-with-no-URL.

2. Docs → fixed. docs/catalog-api.md:476-492 now separates row still_url/still_thumbhash, detail poster_url/poster_thumbhash, and the web hero order apart from the backend row fallback. Matches internal/catalog/detail.go:3231-3232,3252-3253; row fallback in internal/api/handlers/items.go:1345-1350 replaces path+hash together.

3. V1 policy → fixed + pinned. docs/admin-catalog-api.md:486-495 + docs/catalog-api.md:495-503 explicitly rule the change: non-episode titlecard goes from silently overwriting the scope's poster to 400 unsupported_image_type — an allowed bridge correctness fix, not a contract change. admin_images_titlecard_policy_test.go:17-123 pins refusals before image-service work and preserves representative historical mappings (unknown-type fallback, episode coercion). Response shapes unchanged; behavior intentionally tightened.

4. Persistence → fixed. Request-mapping test accurately renamed (admin_images_titlecard_test.go:63-69). New admin_images_titlecard_publication_test.go:77-217 does real DB publication → detail + season-row readback, sibling/parent artwork untouched, parent manual lock checked. V1 still_url omission (policy_test.go:130-147, prod json:"-" at admin_images.go:106-108) + v2 unset-omission (admin_catalog_images_test.go:90-138, omitempty at admin_catalog_images.go:157) pinned. Note the publication test is DB-gated via catalogTransferPool (skips without SILO_TEST_DATABASE_URL) — existence isn't execution proof.

Security/scope/contract all clear: validate-before-download (admin_images.go:349-364 before ApplyItemImage:391), season/episode cache scope intact (:370-399), ClassActingAdmin + DemoRestricted retained, no new downloader (existing private-network guard untouched), OpenAPI adds optional CurrentImages.still_url only, no new IDOR (targets from DB resolution).

Low, non-blocking — PR body is stale

Risks still says "Known follow-up (not in scope): docs/catalog-api.md describes a still_url/detail field…" — that doc is now corrected in-PR. Suggested replacement: "The v2 response change is additive. The shared apply handler intentionally rejects non-episode titlecard requests that previously overwrote the scope's poster; documented request mappings and v1 response shapes remain unchanged." Validation can now mention DB publication + readback. Also policy_test.go:25-27 comment misgroups season backdrop/logo among valid types (cases themselves correct) — tighten when convenient.

Merge gate: body touch-up only, no code change requested.

@randrini
randrini merged commit a58593a into main Oct 7, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v1] Episode-level artwork (title cards)

2 participants