Repository navigation
Conversation
The API's v4 surface keys every response by tvdb_id and treats TVDB as the media's identity. Address by tvdb_id first (tmdb_id and imdb_id still resolve), and read the identity from the response DTO instead of tmdb_id. Episode lookups now carry the series' display order, so a series whose episodes are numbered in DVD or absolute order is resolved by the numbers Emby actually has. Previously a TMDB-addressed series with a non-default order was answered with another episode's segments, silently (Jellyfin watched this happen on Futurama under 'alternate': S07E01 answered default S06E01, whose intro ends at 29.3s instead of 14.6s). Emby's Series.DisplayOrder is an enum rather than Jellyfin's free-form string, so Aired maps to no preference and Dvd/Absolute map to their TVDB order names. Emby cannot express alternate or regional ordering. The episode's own TVDB id is sent as tvdb_episode_id when it differs from the series id. Emby populates provider ids per item and some fetchers repeat the series id on episodes, which the API would read as a non-existent episode; the episode id used to be passed as tvdb_id in that case, which the API reads as a series id. A series with a non-default order but no TVDB id is skipped and logged once - no other id can express the order. Rate limits went up with v4: /media is 120 requests / 10s and 100,000 lookups per day for signed-in callers (was 30 and 1,000) and 5,000 lookups per day anonymous. Pacing now follows the tier in use instead of always assuming the anonymous ceiling. The API key probe moves to /v4/user/stats: v2 and v3 are frozen at GET /media and /v3/user/stats now 404s, which silently disabled key validation.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 5 files. (3 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely identifies the main changes: migration to TheIntroDB API v4 and episode-order-aware lookups. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 5 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
- Key the order-skip warning by the series id, not the episode id, so a large non-default-order series logs one warning instead of one per episode. - Include the pinned TVDB episode id in the not-found cache key: the id outranks season/episode when sent, so a 404 recorded against one id could suppress a later lookup whose id had been corrected within the TTL. The unsupported-display-order finding does not apply here: Emby's Series.DisplayOrder is an enum (Aired, Dvd, Absolute), so no value can reach the API under a name it does not know.
Member
Author
|
All three findings were valid — fixed in
The same two applicable findings are fixed on the Emby PR (#35, commit |
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
1 participant
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.
Moves the plugin to TheIntroDB API v4 and closes the episode-mapping gap reported in TheIntroDB/jellyfin-plugin#31.
Why
/v4keys every response bytvdb_idand treats TVDB as the media's identity. Two things followed from that:tmdb_id, and Emby numbers episodes in the series' display order. Atmdb_idrequest is always read as TMDB numbering, so for a series set to DVD or absolute order the API answered a different episode — silently, and usually plausibly (the credits template matches). Measured on the Jellyfin side of the same API, Futurama underalternateanswered a 14.6 s intro where the file's episode has a 29.3 s one.GET /media, so/v3/user/statsnow 404s and the settings page silently stopped validating keys.What changed
tvdb_idfirst, thentmdb_id, thenimdb_id(the API's own priority).MediaResponsereadstvdb_idinstead oftmdb_id.Series.DisplayOrderis sent asmapping. Emby's is an enum (Aired,Dvd,Absolute) rather than Jellyfin's free-form string, soAiredmaps to no preference (the default order, costing no conversion) andDvd/Absolutemap to their TVDB order names. Emby cannot express alternate or regional ordering at all.tvdb_episode_idwhen it differs from the series id. Emby stores provider ids per item and some fetchers repeat the series id on episodes — the old code passed that through astvdb_id, which the API reads as a series id and would fail to resolve.tmdb_id/tvdb_idfor an episode now come from the series, with IMDb still allowed from either.episode:tvdb:71663:1:1:dvd), so one order's miss can't answer for another's./mediato 120 req / 10 s and 100,000 lookups/day for signed-in callers (was 30 and 1,000), and 5,000/day anonymous. Pacing is now tier-aware: 400 ms anonymous (unchanged), 100 ms with a key./v4/user/stats.Verification
dotnet build: succeeded, 0 errors.dotnet test(net8.0 via the SDK 8 toolchain): 56 passed, including newV4MigrationTestsasserting the exact query strings the client builds and the enum→order mapping.InternalsVisibleToadded to the plugin project so the tests exercise the real members rather than reflecting over private fields.?tvdb_id=…&season=…&episode=…&mapping=dvd&tvdb_episode_id=…,?tvdb_id=169for a movie,400 invalid mappingfor an unknown order), and/v3/user/statsreturns404where/v4/user/statsreturns401without a key.Note for review
tvdb_episode_idoverridesseason/episodeper the v4 docs (confirmed live: sending one episode's id with another's numbers returns the id's episode). We only send it when it is distinct from the series id, so a fetcher that repeats the series id on episodes cannot make the API look for a non-existent episode. If you'd rather send onlymappingand drop the pin entirely, that is a one-line change.No version bump: that's yours.
Summary by CodeRabbit