fix: disambiguate tmdb ids by media type across lookups#2577
Open
fallenbagel wants to merge 6 commits intodevelopfrom
Open
fix: disambiguate tmdb ids by media type across lookups#2577fallenbagel wants to merge 6 commits intodevelopfrom
fallenbagel wants to merge 6 commits intodevelopfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMake mediaType part of unique constraints and lookup keys (Blocklist, Watchlist), change Media.getRelatedMedia to accept items of {tmdbId, mediaType}, update all call sites, remove a Plex scanner guard so shows are always processed, and add migrations to update DB unique indexes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Route as "Route Handler"
participant Service as "Media.getRelatedMedia"
participant DB as "Database"
Client->>Route: Request related media (items: [{tmdbId, mediaType},...])
Route->>Service: getRelatedMedia(user, items)
Service->>DB: SELECT * FROM media WHERE (tmdbId IN finalIds) AND mediaType IN (...)
DB-->>Service: matching media rows
Service->>Route: filtered results (match tmdbId + mediaType)
Route-->>Client: HTTP 200 with related media
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
server/routes/search.ts (1)
38-41: Filter non-movie/tv results beforegetRelatedMedia.This route forwards every multi-search type, but only movie/tv entries need media-status hydration.
Proposed refactor
const media = await Media.getRelatedMedia( req.user, - results.results.map((result) => ({ - tmdbId: result.id, - mediaType: result.media_type, - })) + results.results + .filter( + (result) => + result.media_type === 'movie' || result.media_type === 'tv' + ) + .map((result) => ({ + tmdbId: result.id, + mediaType: result.media_type, + })) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/search.ts` around lines 38 - 41, The route is currently mapping all multi-search results into tmdbId/mediaType and passing them to getRelatedMedia; filter out non-movie and non-tv items first so only entries with media_type === 'movie' or 'tv' are hydrated. Update the logic around results.results.map (and wherever the mapped array is passed into getRelatedMedia) to first do results.results.filter(r => r.media_type === 'movie' || r.media_type === 'tv') then map to { tmdbId: r.id, mediaType: r.media_type } so getRelatedMedia receives only movie/tv items.server/routes/discover.ts (1)
713-716: Avoid coercing non-media trending entries to TV before related lookup.Line 715 maps person/collection results as TV. Filtering to movie/tv first keeps the lookup accurate and avoids unnecessary DB work.
Proposed refactor
- const media = await Media.getRelatedMedia( - req.user, - data.results.map((result) => ({ - tmdbId: result.id, - mediaType: isMovie(result) ? MediaType.MOVIE : MediaType.TV, - })) - ); + const relatedMediaItems = data.results.flatMap((result) => { + if (isPerson(result) || isCollection(result)) { + return []; + } + + return [ + { + tmdbId: result.id, + mediaType: isMovie(result) ? MediaType.MOVIE : MediaType.TV, + }, + ]; + }); + + const media = await Media.getRelatedMedia(req.user, relatedMediaItems);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/discover.ts` around lines 713 - 716, The mapping over data.results currently coerces non-movie entries (people/collections) into MediaType.TV by using isMovie(result) ? MediaType.MOVIE : MediaType.TV; update the logic to first filter results to only movie/tv entries and then map to objects with tmdbId and mediaType so you don't convert people/collections into TV; locate the mapping around data.results.map and use an explicit filter (e.g., keep results where isMovie(result) or isTV(result) / result.media_type === 'movie' || 'tv') before creating the objects with MediaType.MOVIE or MediaType.TV.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/lib/watchlistsync.ts`:
- Around line 70-73: The unavailableItems matching is only comparing tmdbId
which allows movie/show collisions; update the logic that builds and checks
unavailableItems (in watchlistsync.ts where response.items are mapped to objects
with tmdbId and mediaType and where the predicate at or around the
unavailableItems check is used) to include mediaType in the identity check
(i.e., consider both tmdbId and mediaType when storing and when testing
membership) so a TV and a Movie with the same tmdbId do not collide; adjust any
lookups or Set/Map keys to use a composite key or compare both fields.
In `@server/migration/postgres/1772046615085-AddMediaTypeToUniqueConstraints.ts`:
- Around line 6-32: The migration updates only "blocklist" but not "watchlist";
update the same AddMediaTypeToUniqueConstraints migration's up and down to apply
the identical uniqueness change to the "watchlist" table: in up() add queries to
DROP the existing watchlist unique constraint (use the current watchlist
constraint name) and ADD a new UNIQUE constraint on ("tmdbId", "mediaType") for
"watchlist", and in down() reverse those changes by dropping the new watchlist
constraint and re-adding the original single-column UNIQUE on "tmdbId"; keep the
existing blocklist statements and ensure you use the same pattern of SQL strings
inside the class AddMediaTypeToUniqueConstraints' up and down methods so
migrations can be rolled back cleanly.
In `@server/migration/sqlite/1772046587495-AddMediaTypeToUniqueConstraints.ts`:
- Around line 73-75: Remove the legacy global uniqueness constraint on tmdbId
from the final SQLite CREATE TABLE for temporary_blocklist: delete the
CONSTRAINT "UQ_6bbafa28411e6046421991ea21c" UNIQUE ("tmdbId") entry so the only
tmdb uniqueness is the CONSTRAINT "UQ_81504e02db89b4c1e3152729fa6" UNIQUE
("tmdbId", "mediaType"); ensure CONSTRAINT "REL_62b7ade94540f9f8d8bede54b9"
UNIQUE ("mediaId") and the foreign keys remain unchanged and the CREATE TABLE
still defines "mediaType" varchar NOT NULL.
In `@server/routes/watchlist.ts`:
- Around line 61-64: The route calls Watchlist.deleteWatchlist with
req.query.mediaType asserted as MediaType without validation; add an input check
for req.query.mediaType (e.g., a type guard or lookup against the MediaType
enum/allowed values) before invoking Watchlist.deleteWatchlist and respond with
res.status(400).json(...) when mediaType is missing or invalid; update the code
path around Number(req.params.tmdbId) and Watchlist.deleteWatchlist to only run
after the validated mediaType to avoid misleading 404/500 responses.
---
Nitpick comments:
In `@server/routes/discover.ts`:
- Around line 713-716: The mapping over data.results currently coerces non-movie
entries (people/collections) into MediaType.TV by using isMovie(result) ?
MediaType.MOVIE : MediaType.TV; update the logic to first filter results to only
movie/tv entries and then map to objects with tmdbId and mediaType so you don't
convert people/collections into TV; locate the mapping around data.results.map
and use an explicit filter (e.g., keep results where isMovie(result) or
isTV(result) / result.media_type === 'movie' || 'tv') before creating the
objects with MediaType.MOVIE or MediaType.TV.
In `@server/routes/search.ts`:
- Around line 38-41: The route is currently mapping all multi-search results
into tmdbId/mediaType and passing them to getRelatedMedia; filter out non-movie
and non-tv items first so only entries with media_type === 'movie' or 'tv' are
hydrated. Update the logic around results.results.map (and wherever the mapped
array is passed into getRelatedMedia) to first do results.results.filter(r =>
r.media_type === 'movie' || r.media_type === 'tv') then map to { tmdbId: r.id,
mediaType: r.media_type } so getRelatedMedia receives only movie/tv items.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
server/entity/Blocklist.tsserver/entity/Media.tsserver/entity/Watchlist.tsserver/lib/scanners/plex/index.tsserver/lib/watchlistsync.tsserver/migration/postgres/1772046615085-AddMediaTypeToUniqueConstraints.tsserver/migration/sqlite/1772046587495-AddMediaTypeToUniqueConstraints.tsserver/routes/collection.tsserver/routes/discover.tsserver/routes/movie.tsserver/routes/person.tsserver/routes/search.tsserver/routes/tv.tsserver/routes/watchlist.ts
server/migration/postgres/1772046615085-AddMediaTypeToUniqueConstraints.ts
Show resolved
Hide resolved
server/migration/sqlite/1772046587495-AddMediaTypeToUniqueConstraints.ts
Show resolved
Hide resolved
tmdb movie and tv ids can share the same numeric value which can cause cross-contamination in availability status, search results, watchlist, and blocklist lookups. This adds mediaType as a required discriminator to all tmdb id-based queries.
fallenbagel
force-pushed
the
fallenbagel/fix/tmdb-id-media-type-collision
branch
from
9e7d94b to
d045efe
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/routes/discover.ts (1)
713-716: Filter out person/collection before building related-media lookup payload.In trending, non-movie items are currently coerced to TV for lookup. That adds unnecessary DB work and can introduce ambiguous matches for non-TV items.
♻️ Proposed refactor
const media = await Media.getRelatedMedia( req.user, - data.results.map((result) => ({ - tmdbId: result.id, - mediaType: isMovie(result) ? MediaType.MOVIE : MediaType.TV, - })) + data.results + .filter((result) => isMovie(result) || (!isPerson(result) && !isCollection(result))) + .map((result) => ({ + tmdbId: result.id, + mediaType: isMovie(result) ? MediaType.MOVIE : MediaType.TV, + })) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/discover.ts` around lines 713 - 716, The mapping that builds the related-media lookup payload currently coerces any non-movie into MediaType.TV, causing person/collection items to be included; instead, filter data.results first to only keep items that are actual movie or TV entries (e.g., use isMovie(result) || isTV(result) or check result.media_type === 'movie' || result.media_type === 'tv'), then map to { tmdbId: result.id, mediaType: isMovie(result) ? MediaType.MOVIE : MediaType.TV } so person/collection items are excluded and no incorrect TV coercion occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/routes/discover.ts`:
- Around line 713-716: The mapping that builds the related-media lookup payload
currently coerces any non-movie into MediaType.TV, causing person/collection
items to be included; instead, filter data.results first to only keep items that
are actual movie or TV entries (e.g., use isMovie(result) || isTV(result) or
check result.media_type === 'movie' || result.media_type === 'tv'), then map to
{ tmdbId: result.id, mediaType: isMovie(result) ? MediaType.MOVIE : MediaType.TV
} so person/collection items are excluded and no incorrect TV coercion occurs.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
server/entity/Blocklist.tsserver/entity/Media.tsserver/entity/Watchlist.tsserver/lib/scanners/plex/index.tsserver/lib/watchlistsync.tsserver/routes/collection.tsserver/routes/discover.tsserver/routes/movie.tsserver/routes/person.tsserver/routes/search.tsserver/routes/tv.tsserver/routes/watchlist.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- server/lib/watchlistsync.ts
- server/routes/collection.ts
- server/routes/person.ts
- server/lib/scanners/plex/index.ts
- server/entity/Watchlist.ts
- server/routes/movie.ts
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/lib/watchlistsync.ts (1)
79-91: Consider addingmediaTypeto theexistingAutoRequestsquery to reduce over-fetching.Line 84 queries auto-requests by
tmdbId IN (...)without a media-type predicate. When a TMDB ID collision exists (a movie and a TV show share the same numeric ID), both rows are fetched unnecessarily. The composite key inautoRequestedTmdbIds(line 90) keeps the subsequent logic correct, but the query is broader than it needs to be.♻️ Proposed narrowing of the auto-requests query
+ const watchlistItems = response.items.map((i) => ({ + tmdbId: i.tmdbId, + mediaType: i.type === 'show' ? MediaType.TV : MediaType.MOVIE, + })); + + const watchlistTmdbIds = watchlistItems.map((i) => i.tmdbId); - const watchlistTmdbIds = response.items.map((i) => i.tmdbId); const requestRepository = getRepository(MediaRequest); const existingAutoRequests = await requestRepository .createQueryBuilder('request') .leftJoinAndSelect('request.media', 'media') .where('request.requestedBy = :userId', { userId: user.id }) .andWhere('request.isAutoRequest = true') .andWhere('media.tmdbId IN (:...tmdbIds)', { tmdbIds: watchlistTmdbIds }) + // Further narrow to only the media types present in the watchlist + .andWhere('media.mediaType IN (:...mediaTypes)', { + mediaTypes: [...new Set(watchlistItems.map((i) => i.mediaType))], + }) .getMany();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/watchlistsync.ts` around lines 79 - 91, The query that builds existingAutoRequests fetches rows by tmdbId only and can over-fetch when the same numeric TMDB ID exists for different media types; update the requestRepository.createQueryBuilder('request') call to also filter by media.mediaType (e.g. add an .andWhere('media.mediaType = :mediaType', { mediaType }) or, if handling multiple types, split tmdbIds by mediaType and query per type) so the result set matches the watchlist item types before producing autoRequestedTmdbIds; adjust the call-site to supply the corresponding mediaType(s) when building the query and keep the join on 'media' as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/lib/watchlistsync.ts`:
- Around line 79-91: The query that builds existingAutoRequests fetches rows by
tmdbId only and can over-fetch when the same numeric TMDB ID exists for
different media types; update the
requestRepository.createQueryBuilder('request') call to also filter by
media.mediaType (e.g. add an .andWhere('media.mediaType = :mediaType', {
mediaType }) or, if handling multiple types, split tmdbIds by mediaType and
query per type) so the result set matches the watchlist item types before
producing autoRequestedTmdbIds; adjust the call-site to supply the corresponding
mediaType(s) when building the query and keep the join on 'media' as-is.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the backend to treat TMDB IDs as only unique within a media type (movie vs TV), ensuring lookups and uniqueness constraints don’t collide across types.
Changes:
- Add
mediaTypediscrimination to “related media” lookups and watchlist checks across multiple routes. - Update watchlist deletion to require and validate
mediaType, and return404when an entry isn’t found. - Update DB uniqueness constraints (blocklist/watchlist) to include
mediaType, and remove the Plex scanner’stvdbIdguard for show processing.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/routes/watchlist.ts | Requires/validates mediaType for delete and maps not-found to 404. |
| server/routes/tv.ts | Adds mediaType: tv to watchlist existence check and related-media lookups. |
| server/routes/movie.ts | Adds mediaType: movie to watchlist existence check and related-media lookups. |
| server/routes/search.ts | Passes { tmdbId, mediaType } into related-media lookup for multi-search results. |
| server/routes/person.ts | Filters combined credits to items with media_type and passes { tmdbId, mediaType } into related-media lookup. |
| server/routes/discover.ts | Passes typed { tmdbId, mediaType } items into related-media lookup across discover endpoints. |
| server/routes/collection.ts | Treats collection parts as movies when building related-media lookup items. |
| server/entity/Media.ts | Changes getRelatedMedia signature to accept typed { tmdbId, mediaType }[] and filters results by both fields. |
| server/entity/Watchlist.ts | Expands unique constraint to include mediaType; updates deleteWatchlist to require mediaType. |
| server/entity/Blocklist.ts | Updates uniqueness and media lookup to include mediaType. |
| server/lib/watchlistsync.ts | Disambiguates TMDB collisions by including mediaType in matching logic and keys. |
| server/lib/scanners/plex/index.ts | Always calls processShow (allows missing TVDB ID). |
| server/migration/sqlite/1772047972752-AddMediaTypeToUniqueConstraints.ts | SQLite migration for composite unique constraints on blocklist/watchlist. |
| server/migration/postgres/1772048000333-AddMediaTypeToUniqueConstraints.ts | Postgres migration for composite unique constraints on blocklist/watchlist. |
Comments suppressed due to low confidence (1)
server/entity/Watchlist.ts:156
deleteWatchlistthrowsnew NotFoundError('not Found'), which results in an inconsistent/odd error message being returned by the API. Consider using the default message (new NotFoundError()) or changing it to consistent casing (e.g. "Not found").
if (!watchlist) {
throw new NotFoundError('not Found');
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
server/migration/sqlite/1772047972752-AddMediaTypeToUniqueConstraints.ts
Show resolved
Hide resolved
server/migration/sqlite/1772047972752-AddMediaTypeToUniqueConstraints.ts
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
fallenbagel
force-pushed
the
fallenbagel/fix/tmdb-id-media-type-collision
branch
from
f2c4e0a to
3f1466f
Compare
…NotFoundError status to 404
gauthier-th
approved these changes
Mar 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
TMDB IDs are only unique within their media type (i.e., a movie and a TV show can share the same numeric ID like 286801). Seerr was treating TMDB IDs as globally unique in several places: the Plex scanner skipped TV shows lacking a TVDB ID entirely, getRelatedMedia matched by ID alone causing status cross-contamination, and the blocklist/watchlist entities had insufficient uniqueness constraints.
This PR adds mediaType as a discriminator wherever TMDB IDs are used for lookups, and removes the unnecessary tvdbId guard in the Plex scanner that prevented shows from being marked as available.
Note
The generated migrations were manually edited because TypeORM's
migration:generatedoes not detect changes to named unique constraints(watchlist's
UNIQUE_USER_DB), and its SQLite's differ incorrectly retained the legacy single-columnUNIQUE("tmdbId")constraint on blocklist alongside the new composite one.How Has This Been Tested?
Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
Bug Fixes
New Features
Chores