feat(watchsync): sync ratings with watch-sync plugins - #1357
Merged
Merged
Conversation
Trakt and Simkl answered every non-2xx the same way, so a 429 never became a RateLimitedError and the sync service never deferred the account. Both providers now map 429 (and Simkl's documented rate_limit/RATE_LIMIT bodies) to RateLimitedError with the provider's Retry-After, retry short waits in place, and pace authenticated writes to one per second per access token as the providers document. Retry-After parsing and the per-credential write limiter move into internal/watchsync so MDBList, Trakt and Simkl share them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Since April 2026 Trakt serves only the first 100 items when page and limit are omitted. Favorites, watchlist, and history reads sent neither, so users with longer lists got partial imports, and ExportWatched compared local plays against a truncated history and re-sent older plays, which Trakt stores as duplicates. Generalize the watched-only pager into fetchTraktPages and use it for watched, favorites, watchlist, and history. It always sends limit=250, stops on X-Pagination-Page-Count, a page shorter than the applied X-Pagination-Limit, or an empty page, caps the walk, and returns no rows when any page fails. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The MDBList watch-sync provider puts the API key in the request query string. Transport errors wrap *url.Error, whose message includes that URL, and those errors reach the connection's last_error, sync run errors shown in the web UI, and logs. Build the keyed URL only inside the single request helper, sanitize transport and request-build errors with logredact.SanitizeURLError, and mask the key (raw or escaped) in error-body excerpts. Cancellation and timeout classification is kept. Same bug class as #692, which covers the separate internal/mdblist discovery client. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Trakt favorites and watchlist writes mapped not_found echoes back to request items by one derived key per side. Silo keys items IMDb-first while the provider keyed shows TVDB-first, so a show sent with both ids and echoed as missing was still reported as sent. Match echoes by any shared identifier (Trakt id, slug, IMDb, TMDB, TVDB), namespaced by kind because TMDB and TVDB number movies and shows separately. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Silo profiles rate movies and series with 1-5 stars, but watch-provider sync never carried ratings. Add rating sync with separate import and export settings per connection; new connections get both on, existing connections start with both off. Each item is a three-way merge of the local rating, the provider rating (integers 1-10, converted to stars by rounding half up), and the last rating both sides agreed on, stored in watch_provider_rating_items. The side that changed wins; a conflict never deletes and otherwise the newer change wins. Missing items count as provider removals only in complete snapshots of their kind, when no read row shares one of their ids, and when a previous read confirmed the provider held the rating. Imports are compare-and-set against the observed local value, so concurrent edits and multi-node runs stay safe without a lock. Local rating changes are sent through a value-less event from the shared rating seam. Settings and run counters are exposed on /api/v2 and in the web settings page; the frozen v1 responses and settings update are unchanged apart from two additive capability flags. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Read every Trakt movie and show rating through the paged reader and report both as complete snapshots. Send ratings to /sync/ratings with the local rating time and clear them through /sync/ratings/remove, mapping not_found echoes back to items by any shared id. Trakt already uses the 1-10 scale, so values pass through unchanged. Season and episode ratings are left alone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Require the silo-plugin-sdk commit that adds rating state and the series media type. Bump to the v0.17.0 tag once it is released. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Plugin providers can now advertise import_ratings and export_ratings. The adapter reads RATING remote state (a complete snapshot covers the rateable kinds the plugin supports; an incremental read covers none, so absent titles stay unknown), decodes key-only tombstones, and sends SET_RATING and REMOVE_RATING events. A SET event id carries the rating and rating time, so a retry reuses it and a later change does not. Series now cross the plugin boundary as the SDK's SERIES media type, for ratings and for favorites and watchlist; plugins that do not list SERIES see no change, and SERIES watched or progress rows are rejected because the contract does not define them. A plugin rates only the kinds it supports, so a movie-only plugin is never sent series ratings. Media types from a newer SDK are ignored instead of rejecting the plugin, so a future plugin release that opts into a new type keeps syncing on servers that predate it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Quick104 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This was referenced Sep 24, 2026
![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: f9fce03520
ℹ️ 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".
A provider 429 during a watch-provider call, such as Trakt's slow_down answer to a device-code poll, now reaches clients as a rate-limited problem carrying the provider's wait instead of an internal error. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
One title added and another removed between page requests keeps the item count unchanged while shifting page boundaries, so a row can be skipped without any header changing. A listing that spans several pages is now read twice, and the read fails unless both passes return the same rows. Single-page listings are read once. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Address review findings on the rating sync core: - The shared capabilities object is serialized by the frozen /api/v1 responses, so its rating flags were leaking into v1. v2 now projects its own WatchProviderCapabilities type (same schema name and fields, so the v2 contract is unchanged) and the shared type hides the flags. - A sync still in flight when the connection is re-bound to another provider account (possibly on another node) no longer applies the old account's ratings. - The import compare-and-set also compares rated_at, so re-saving the same stars after the sync read them still wins. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A complete RATING snapshot containing a nil item or a state without a rating payload silently dropped it and still claimed every supported kind, so a still-rated title could read as removed. Such rows now make the snapshot cover no kind, with a warning, like other unreadable rows. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…mdblist-key-redaction
…-trakt-pagination
Trakt allows 500 authenticated GETs per five minutes. Reading a large listing twice could exceed that, and the 429 discarded both passes, so a very large account retried from page 1 without ever finishing. Paged reads are now paced per token: a burst of 50 pages, then one every 675 ms, which keeps any five-minute window under the limit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A local limiter refuses at once when the next slot lies past the context deadline. That was an ordinary error, so watched exports that were never sent were marked failed. The refusal is now a RateLimitedError, which leaves the work pending for a later run; a cancelled context still returns its own error. MDBList's limiter gets the same treatment. Starting Trakt device authorization now reports a 429 as a rate limit with the provider's Retry-After, like polling and token refresh. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
When a transport error quoted the keyed URL in its own message, the masking fallback replaced it with a plain error, so errors.Is and errors.As no longer matched cancellations, deadlines, or net.Error timeouts. The masked error now answers Is and As from the original, without an Unwrap that would expose its message. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nged mid-send Rating cursors now update with a jsonb patch conditioned on the bound account instead of rewriting the whole connection row, so a concurrent settings edit or rebind is not overwritten. A confirmed set or removal that finds the local rating changed during the send resends the current value once. An account rebind drops other accounts' agreed rows only after the new binding is saved. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…mdblist-key-redaction
…-trakt-pagination Also routes the Trakt page limiter's refused waits through LimiterWaitError so they defer the sync like a 429. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…until settled Agreed-rating upserts and deletes now apply only while the connection is still bound to the row's provider account, so a run that outlived a rebind can't take rows back from the new account. Ratings that change while their write is in flight are resent up to three times; if they still haven't settled, the agreed row is dropped so the next merge keeps the newer rating instead of importing a stale provider value. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…te removals Agreed-rating upserts now share-lock the connection row and check the bound account under that lock, so a rebind either waits for the write and clears it or the write finds no binding. When resends run out, the agreed row records the value last confirmed on the provider, so a removal made during the last resend is sent next run instead of being imported back. The web settings page now refreshes rating, catalog, section and recommendation queries when the manual sync it started finishes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…agination # Conflicts: # internal/watchsync/providers/trakt/provider.go
…rating-sync # Conflicts: # contracts/api/v2/fixtures/get_system_info_ok.json
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a2e7caca1
ℹ️ 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".
Comment on lines
+187
to
+188
| if !row.Removed && !ratingSyncKind(row.Kind) { | ||
| continue |
There was a problem hiding this comment.
When a plugin advertises only MOVIE support but returns a SERIES rating, this condition accepts the row because ratingSyncKind(series) is true. Although dropUnsyncedRatingKinds initially removes local series items, resolveRemoteRatings later recreates matched remote-only items, so the unsupported row can import or overwrite a series rating. Filter non-tombstone rows through p.supportsMedia(watchSyncMediaType(row.Kind)) before adding them to the batch.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.
Problem
Related issue: N/A
Validation tasks: none
Stack: builds on #1354; merge that first.
Watch-sync plugins couldn't take part in rating sync. The plugin contract also had no series media type, so series favorites, watchlist entries and ratings couldn't cross the plugin boundary.
Approach
This builds on the SDK change Silo-Server/silo-plugin-sdk#23 and requires plugin SDK v0.17.0.
Capabilities. Plugin
import_ratingsandexport_ratingsmap to the rating capabilities.Reads.
RATINGremote state.Writes.
SET_RATINGandREMOVE_RATINGevents are sent.REJECTEDremoval counts as a failure and is retried, because the contract says removing an absent rating must succeed.Series.
SERIES, for ratings and for favorites and watchlist.SERIESsee no change.Forward compatibility. Media types from a newer SDK are ignored instead of rejecting the plugin. A later plugin release that opts into a new type then keeps syncing on servers that predate it. This is exactly what would break older servers with the new Floppy manifest.
Validation
SERIESin both directions;go test ./internal/watchsync/... ./internal/plugins/...passes.make lint-changedreports 0 issues.Risks
SERIESor the rating flags. Plugin releases that use them must follow a server release that includes this PR.Checklist
AI Disclosure
🤖 Generated with Claude Code