feat: media library backend with five browse lenses#542
Conversation
…tems Add the @ent-mcp/shared/library subpath (lens/watched/quality enums, facet and collection types, lens/collections query schemas), an additive optional collection field on the plugin-sdk media item, collection membership on the shared CanonicalMetadata type, CompactMediaItem.section for grouped lenses, and the library lens source ids. Includes the library backend design doc.
Map a movie's belongs_to_collection into the canonical metadata: tmdb mapMovie emits a stringified collection (TV maps to null), the catalog mapper persists collectionId/collectionName, and writeMetadata keeps them on a conflict update so a re-fetch refreshes franchise data. Adds the two canonical_metadata columns.
Add the library-owned library_items (composite (user_id, id) primary key so the same title can be owned by many users) and user_library_seed tables, the 0004 migration (also adding the canonical_metadata collection columns), and the server-mod-library / -internal / server-schema-library fallow zones plus the client-feat-library allows for the shared media and virtualized grid.
…ets, collections New library module: membership sync from collection@v1 (tombstone, no-resurrect, no wipe on a partial/empty feed), denormalized hydrate from catalog + availability + progress, the az/timeline/server/quality lens sources wired into the unified media REGISTRY, and the /api/library facets + collections endpoints. Adds MediaService.getCollectionFeed and getAvailabilityQuality. Registers the library.sync (6h) and library.hydrate (hourly) jobs. Tests cover sync diff, keyset stability, json_each fan-out, facets, hydrate, and multi-user ownership.
Replace the mock library data layer with the real endpoints (look preserved): the four item lenses reuse the shared media-rows infinite query against /api/media/sources/library-<lens>, collections and facets via /api/library. Section headers insert on group-key change over the flat stream (A-Z letter, timeline decade, server/quality section), rows key by id+section, infinite scroll via the shared virtual grid. Quality chips render from item tags; A-Z and decade rails are facet-driven. Drops the fixtures, fetch-all stub, and client-side grouping/filtering.
github-actions
Bot
added
the
status: ready-for-review
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91cd76f11f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fallow combined reportFound 69 findings. Duplication (52, showing 50)
Showing 50 of 52 findings. Run fallow locally or inspect the CI output for the full report. Health (16)
Architecture (1)
Generated by fallow. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -0,0 +1,40 @@ | |||
| import { clearSeedLock, trySeedLock } from "../repo"; | |||
| import { syncMembership } from "../service"; | |||
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| db: Db = getDb(), | ||
| ): Promise<number> { | ||
| const base = and(eq(libraryItems.userId, userId), eq(libraryItems.owned, true)); | ||
| const where = keepKeys.length > 0 ? and(base, notInArray(libraryItems.id, keepKeys)) : base; |
There was a problem hiding this comment.
Non-blocking — footgun in the function surface.
When keepKeys is empty, where collapses to just base (userId = X AND owned = true), which would tombstone the entire library. The outer guard in syncMembership (feedKeys.length === 0 → skip) prevents this today, but the function is exported and the danger is invisible to future callers. Suggest a hard guard at the top:
| const where = keepKeys.length > 0 ? and(base, notInArray(libraryItems.id, keepKeys)) : base; | |
| if (keepKeys.length === 0) return 0; | |
| const base = and(eq(libraryItems.userId, userId), eq(libraryItems.owned, true)); | |
| const where = and(base, notInArray(libraryItems.id, keepKeys)); |
| const keys = targets.map((target) => ({ tmdbId: target.tmdbId, type: target.mediaType })); | ||
| try { | ||
| return await ctx.catalog.getMetadataBatch(keys); | ||
| } catch (err) { |
There was a problem hiding this comment.
Non-blocking — unbounded concurrency.
Promise.all fans out every stale target simultaneously. Each buildUpdate fires 2 concurrent plugin calls (getMatchingServers + getAvailabilityQuality), so a user with 500 stale rows spawns 1000 concurrent plugin requests in one job tick. This is a background job so the timing is acceptable, but it can spike the plugin adapter's connection pool on a large initial seed.
Cheap mitigation: chunk the fan-out (e.g., 50 at a time with pLimit or a manual slice loop). Not required for merge, but worth a followup issue before the first 10k+ library user hits the hourly pass.
| * them. An omitted axis stays undefined → the repo applies no filter for it. | ||
| */ | ||
| function toLensParams(params: LibraryLensQueryParsed): { filters: LensFilters; limit: number } { | ||
| const filters: LensFilters = {}; |
There was a problem hiding this comment.
Non-blocking — multi-value filter axes silently collapse (acknowledged in PR).
toLensParams receives params already parsed through libraryLensQuerySchema. The schema's arrayParam() helper coerces a single string to a one-element array and handles repeated query params correctly — but only when the values actually arrive as an array. Because the shared /media/sources/:id resolver reads params via c.req.query() (returns a single string for a repeated key, not an array), a request like ?genres=Action&genres=Comedy delivers "Comedy" not ["Action","Comedy"] to the schema, which coerces it to ["Comedy"].
Collections and facets endpoints are unaffected (they use zValidator directly). The fix is one line in the shared resolver (c.req.queries() → array). Worth a dedicated PR soon since it silently narrows multi-axis filters for every item lens.
| ); | ||
| --> statement-breakpoint | ||
| ALTER TABLE `canonical_metadata` ADD `collection_id` text;--> statement-breakpoint | ||
| ALTER TABLE `canonical_metadata` ADD `collection_name` text; No newline at end of file |
There was a problem hiding this comment.
Non-blocking — missing newline at end of file. Some migration runners (and diff tooling) warn on this. Trivial fix:
| ALTER TABLE `canonical_metadata` ADD `collection_name` text; | |
| ALTER TABLE `canonical_metadata` ADD `collection_name` text; |
(just add a trailing newline after the semicolon)
| /** | ||
| * Folds the three sources into one row's denormalized projection. The | ||
| * availability lookups (`getMatchingServers`, `getAvailabilityQuality`) are the | ||
| * only awaits here; metadata and progress are already loaded. Each source is |
There was a problem hiding this comment.
Non-blocking — silent partial on progress failure.
loadProgress swallows errors and returns an empty map. The hydrate result (HydrateResult) has no partial field, so a total CW failure produces a batch of rows with watchedState: null that look fully hydrated to the caller — the job run status shows hydrated: N with no sign of degradation. Consider either surfacing partial in HydrateResult or at minimum emitting a warn log here rather than only inside deriveWatchedState.
Code Review — PR #542: media library backend + five browse lensesDesign-doc sync ✅Design doc Fallow baseline ✅
Changesets ✅Four changesets: What the PR doesImplements the full library backend end-to-end:
SecurityNo injection or auth gaps found. All routes sit behind Performance
TestingExtensive. New suites: sync diff/idempotency/no-resurrect/no-wipe-on-outage, franchise mapping + persist, lens keyset stability (NULL-year, null-name boundaries), Code quality
Issues raised inline
None of these are blocking. Approved to merge after addressing the |
github-actions
Bot
removed
the
status: ready-for-review
2 participants
Summary
Builds the backend for the owned-media library and rewires the existing (mock-only) library page to it across five browse lenses — A–Z, Timeline, Server, Quality, and Collections — implementing
docs/2026-06-02-library-backend-design.mdend to end. A newlibraryserver module owns a denormalizedlibrary_itemsprojection synced fromcollection@v1, served through the unified media-sources route plus dedicated facets/collections endpoints; the client swaps its fixtures for real data with infinite scroll while keeping the existing look.Design document
Implements
docs/2026-06-02-library-backend-design.md(updated in this PR with per-phase status, deviations, and a bug ledger — code and doc are in sync).Type of change
Scope
@ent-mcp/client@ent-mcp/server@ent-mcp/sharedAlso
@ent-mcp/plugin-sdkand@ent-mcp/plugin-tmdb(collection membership).What's here
@ent-mcp/shared/library(lens/watched/quality enums, facet + collection types, query schemas);collectionon the media item;collectionId/NameonCanonicalMetadata;CompactMediaItem.sectionfor grouped lenses.belongs_to_collection→mapMovie(TV → null) → canonical metadata, persisted on insert and conflict-update.library_items(composite(user_id, id)PK) +user_library_seed; migration0004;server-mod-library/-internal/server-schema-libraryfallow zones.REGISTRY, and/api/library/{facets,collections}.library.sync(6h) +library.hydrate(hourly) jobs./api/library/*, section headers on group-key change,id+sectionkeys, virtualized infinite scroll, quality chips, facet-driven A–Z + decade rails. Mock fixtures and client-side grouping/filtering removed.Test plan
vp checkpasses locally — 1553 files, no lint/type errors.vp testpasses locally — full monorepo 2674 tests; library suites green (server + client).fallow dead-code→ 0 boundary violations, baseline unchanged.New regression tests cover: sync diff/idempotency/no-resurrect/no-wipe-on-outage, franchise mapping + persist, lens keyset stability (incl. NULL-year and null-name boundaries),
json_eachserver/quality fan-out, facet counts, hydrate, multi-user ownership, and the FE hooks/section grouping/infinite-scroll/collections/facets.Adversarial review across phases caught and fixed 11 real bugs (incl. an outage wiping the owned library, a multi-tenancy PK collision, two keyset row-drop blockers, and a
json_eachruntime error) — full ledger in the design doc.Known followup
Multi-value filter axes on the item lenses collapse to the first value because the unified
/media/sources/:idresolver readsc.req.query()(single-value); collections + facets already honor multi-value. Server-side fix: readc.req.queries().Checklist
docs/2026-06-02-library-backend-design.md)@ent-mcp/server,@ent-mcp/client,@ent-mcp/plugin-sdk,@ent-mcp/plugin-tmdb)