Skip to content

[script.punchplay] 1.5.2 - #2884

Open
PunchPlay wants to merge 1 commit into
xbmc:omegafrom
PunchPlay:omega
Open

[script.punchplay] 1.5.2#2884
PunchPlay wants to merge 1 commit into
xbmc:omegafrom
PunchPlay:omega

Conversation

@PunchPlay

@PunchPlay PunchPlay commented Aug 23, 2026

Copy link
Copy Markdown

Changes

Updates the addon from 1.3.0 (currently on the official repo) to 1.5.2. Summary of what changed across that range:

Sync

  • Two-way sync: "Sync From PunchPlay Now" applies PunchPlay watched history (playcount + lastplayed) and resume points to the Kodi library, matched by TMDB/IMDb ids. Never un-watches local items, and a stale remote resume position can no longer overwrite an item already finished locally.
  • Optional auto-sync from PunchPlay every 6 hours (incremental, off by default), with per-item retry so one persistently failing item can't block newer changes forever, and checkpoints that reset after an account or setting change.
  • Live watched sync: manually marking a movie/episode watched in the Kodi library now syncs to PunchPlay within seconds, with echo suppression so our own pull-sync and playback writes aren't re-imported. Un-watching an item within the same debounce window as marking it watched now correctly cancels the pending upload instead of still sending it.
  • Library import now sends per-item playcounts (rewatches) and converts local-time lastplayed to UTC before upload.
  • Import batch size raised from 50 to 100 (the backend maximum), fixing large libraries that previously stopped importing after roughly 5,000 items. A sync that hits errors reports how many items failed instead of claiming success, and stops after three consecutive failed batches instead of grinding through an unreachable backend.

Reliability

  • Fixed the token-refresh request being rejected outright by the backend's network edge — it was missing the User-Agent header every other request sends, which silently broke reconnection for every user once their access token expired and forced a manual re-login each time.
  • Fixed intermittent HTTP 401 errors during sync: token refresh is serialized behind a lock so a heartbeat and a background sync racing on an expired token can no longer invalidate each other's retry.
  • Fixed playback left "now playing" on the server forever if Kodi was closed while something was still playing — shutdown now always sends the stop event instead of only clearing local state, and preserves queued/in-flight playback events for ordered offline replay.
  • Scrobble events are now sent from a background worker instead of on Kodi's player callback thread, so a slow or unreachable backend can no longer stall play/pause/resume.
  • Logout discards queued and in-flight work from the previous account instead of letting it retry with the wrong account's credentials.
  • Offline queue entries are dropped after ~1 day of continuous failed retries (previously up to 30 days).

Ratings

  • Rating prompts no longer block Kodi's player callbacks; the approval poll backs off correctly when the server rate-limits it.
  • "Never for this show" suppression is now keyed on the show's canonical id instead of its per-episode year, so it no longer stops matching once a show crosses into a new year.
  • Rating prompts can now be limited to movies without disabling episode scrobbling or watched sync.

113 unit tests passing, kodi-addon-checker --branch omega clean.

Checklist:

  • My code follows the add-on rules and piracy stance of this project.
  • I have read the CONTRIBUTING document
  • Each add-on submission should be a single commit with using the following style: [script.foo.bar] 1.0.0

@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

Greptile Summary

The PR upgrades PunchPlay from 1.3.0 to 1.5.2 with two-way watched-state synchronization and substantial playback, authentication, queueing, and rating reliability changes.

  • Adds PunchPlay-to-Kodi history and resume synchronization plus live Kodi watched-toggle uploads.
  • Serializes token refresh and pins queued work to the authentication generation.
  • Moves network writes onto ordered background processing and improves offline replay and shutdown handling.
  • Adds configurable rating scope and migrates rating-prompt suppressions.

Confidence Score: 4/5

The PR is not yet safe to merge because a manual watched toggle made shortly after playback can still be silently omitted from PunchPlay.

The current echo guard removes a queued same-item watched update based solely on proximity to the playback timestamp, so a legitimate manual update within the five-second matching window remains indistinguishable from Kodi’s playback-generated update and is discarded.

Files Needing Attention: script.punchplay/resources/lib/library_events.py and script.punchplay/resources/lib/player.py

Important Files Changed

Filename Overview
script.punchplay/resources/lib/library_events.py Adds live watched-toggle batching and echo suppression, but the previously reported timestamp-only suppression defect remains.
script.punchplay/resources/lib/player.py Adds asynchronous ordered playback posting and supplies recent-item timestamps consumed by the outstanding echo-suppression path.
script.punchplay/resources/lib/pull_sync.py Implements PunchPlay-to-Kodi watched and resume synchronization with incremental checkpoints and per-item retries.
script.punchplay/resources/lib/api.py Adds serialized token refresh, authentication generations, account-pinned replay, and device-login backoff.
script.punchplay/resources/lib/service.py Integrates live and pull synchronization with the shared background network worker and service lifecycle.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  K[Kodi library update] --> L[LiveWatchedSync queue]
  P[Playback timestamp] --> E[Echo suppression]
  U[Pull-sync write timestamp] --> E
  L --> E
  E -->|accepted| B[Build import entry]
  B --> W[Background post worker]
  W --> A[PunchPlay API]
  A -->|failure| Q[Offline queue]
  Q --> W
Loading

Reviews (2): Last reviewed commit: "[script.punchplay] 1.5.2" | Re-trigger Greptile

Comment thread script.punchplay/resources/lib/library_events.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 088441dfc4

ℹ️ 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 thread script.punchplay/resources/lib/pull_sync.py Outdated
@PunchPlay

PunchPlay commented Aug 23, 2026

Copy link
Copy Markdown
Author

both findings were real bugs, not false positives. Fixed and force-pushed:

  • Recent-playback echo suppression dropping manual watches: LIVE_SYNC_RECENT_PLAY_WINDOW_SECS was 600s (10 min), meant only to bridge Kodi's own near-immediate playcount write after our stop scrobble. Cut to 30s so it can't also swallow a genuine manual "mark as watched" made minutes later.
  • Resume-only writes suppressing later watched toggles: run_pull_sync's apply_resume branch was adding synced items to applied_out even though a resume write never touches playcount. Removed — only genuine watched-state writes populate the echo-suppression set now. Added a regression test (test_resume_only_sync_does_not_populate_applied_out) covering this.

Also shrank LIVE_SYNC_PULL_APPLIED_SUPPRESS_SECS the same way for consistency (same class of issue, same reasoning).

kodi-addon-checker --branch omega and the full unit suite (74 tests) both pass clean against the updated tree.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 601c774ff9

ℹ️ 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 thread script.punchplay/resources/lib/player.py Outdated
Comment thread script.punchplay/resources/lib/pull_sync.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a67a9c098

ℹ️ 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 thread script.punchplay/resources/lib/service.py
@PunchPlay

Copy link
Copy Markdown
Author

Two more real findings from the Codex review, both fixed and force-pushed:

  • Echo suppression not covering untracked plays: correct — onAVStarted stamps _current_library_item before identify()/min_length filtering runs, but the stop-time restamp only fired inside _handle_stop's tracked-scrobble path. Once the "recently played" window was cut to 30s (previous fix), a filtered-out play running longer than that would leak Kodi's own playcount bump into live sync as if it were a manual toggle. The stamp-and-clear now runs unconditionally at the top of _handle_stop, before the _metadata is None guard.
  • Pull sync checkpoint advancing past failed writes: correct — a per-item VideoLibrary.Set*Details failure was only logged, never counted, so the incremental checkpoint still advanced and the next since-filtered sync would treat that item as already covered. Failures are now counted in the summary (apply_failed), and the service loop holds the checkpoint back when any occurred so the item gets retried next time instead of being permanently skipped.

Regression tests added for both. kodi-addon-checker --branch omega and the full unit suite (76 tests) pass clean against the updated tree.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4cf315a314

ℹ️ 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 thread script.punchplay/resources/lib/player.py
@PunchPlay

Copy link
Copy Markdown
Author

Updated once more after a deeper self-review of the full diff surfaced a few more real issues before they could show up as further review comments here:

  • The pull-sync checkpoint fix from the last update could, in the worst case, hold back forever if one item kept failing every run. Retry counts are now tracked per failed item (a stable hashed identity, no raw metadata persisted) so a single persistently-bad item can't block newer items indefinitely, and an unrelated new failure doesn't inherit another item's exhausted retry allowance.
  • The shutdown-time job persistence added last update had its own edges: the join timeout didn't reliably cover a 401-refresh-retry chain, the drain raced the worker thread for the same queue, and — the sharpest one — a later event could be persisted to the offline queue before an earlier one still in flight, reversing replay order. Fixed by sorting offline-queue replay by each event's own timestamp instead of insertion order, having shutdown explicitly snapshot and persist the in-flight job (safe to double-persist, since it's replayed idempotently by the same event_id backend dedup used elsewhere), and shrinking the join timeout back to a short grace period now that correctness no longer depends on it.
  • Two small cleanups: a dead optional parameter, and two echo-suppression window constants that duplicated one concept.

kodi-addon-checker and the full unit suite (89 tests) pass clean against the updated tree.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b94242354

ℹ️ 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 thread script.punchplay/resources/lib/player.py Outdated
Comment thread script.punchplay/resources/lib/service.py Outdated
@PunchPlay

Copy link
Copy Markdown
Author

Updated once more after a further review pass against this branch's actual baseline. Additional fixes in this push:

  • Logout now discards queued and in-flight work for the account being logged out, including a delayed rating prompt that could otherwise fire under the wrong account.
  • Transient watched-toggle write failures (a locked library.db, a momentary RPC error) are requeued with a short retry delay instead of being dropped on the first attempt.
  • Echo suppression now keys off actual write timestamps rather than a fixed window, so it can't drop a genuine manual watch that happens to land near the edge of the suppression period.
  • Pull-sync checkpoints reset when the linked account or the enabled sync options change, so switching accounts or toggling watched/resume sync doesn't skip a window of real changes.
  • The QR-code and manual-code-entry login pollers can no longer race to consume the same one-time device code.
  • "Never ask again for this show" now applies to every episode of the show, not just the one it was set from.
  • Pull-sync progress reporting only counts the operations actually enabled in settings.

kodi-addon-checker and the full unit suite (99 tests) pass clean against the updated tree.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8ad89b5a84

ℹ️ 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 thread script.punchplay/resources/lib/api.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 700b699e96

ℹ️ 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 thread script.punchplay/resources/lib/player.py
Comment thread script.punchplay/resources/lib/service.py Outdated
@PunchPlay

Copy link
Copy Markdown
Author

Found and fixed a significant one in this update, confirmed against a real user report and reproduced locally.

The token-refresh request built its own bare headers and was the only outbound call in the addon skipping the User-Agent every other request sends. A bare Python-urllib/x.x is a common bot-mitigation signature, and edge protection in front of the backend was rejecting every single refresh attempt with a 403 — not intermittently, every time, regardless of how correct the surrounding refresh logic (locking, coalescing, retry) was. In practice this meant the access token (1 hour lifetime) could never actually renew itself, forcing a full manual relogin every time it expired.

Reproduced locally with a token that had been expired since the previous day's login: before the fix, every scrobble attempt failed and queued, on a fresh restart, with the 403 visible in the log on every retry. After routing the refresh call through the same header-building path as everything else, the identical expired-token scenario refreshed silently with no relogin needed.

Also included: a guard against a request from a since-replaced login (different account, or logout) retrying with the wrong account's credentials after a refresh.

kodi-addon-checker and the full unit suite (107 tests) pass clean against the updated tree.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d04625028

ℹ️ 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 thread script.punchplay/resources/lib/player.py Outdated
Comment thread script.punchplay/resources/lib/pull_sync.py Outdated
@PunchPlay

Copy link
Copy Markdown
Author

One more real bug found and fixed, confirmed live: quitting Kodi while something was playing never sent the stop event.

onPlayBackStopped/onPlayBackEnded only fire when playback ends while Kodi keeps running — a whole-app quit only ever reaches PunchPlayPlayer.cleanup(), called from the service's shutdown path. cleanup() was clearing local playback state directly instead of going through the same _handle_stop() path every other stop condition uses, so it never actually emitted the stop POST. The item was left stuck in "now playing" on the backend indefinitely — for every user, every time they closed Kodi mid-playback, independent of anything else in this PR.

Fixed by routing cleanup() through _handle_stop() like every other stop path, so it benefits from the same shutdown-persistence handling already in this PR (offline queue on a slow/unreachable backend, ordered replay) rather than a separate, incomplete code path.

Verified live: played a title, quit Kodi mid-playback, confirmed the stop event (with correct position) now appears in the log immediately before shutdown — previously nothing was logged at all for that case.

kodi-addon-checker and the full unit suite (108 tests) pass clean against the updated tree.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1178ad80fb

ℹ️ 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 thread script.punchplay/resources/lib/player.py Outdated
Comment thread script.punchplay/resources/lib/service.py
@PunchPlay

PunchPlay commented Aug 24, 2026

Copy link
Copy Markdown
Author

Went back through the review comments on this PR to check which findings were still genuinely open against the current tree. Two were still real:

  • "Never for this show" rating suppression breaking across season/year boundaries: the show-level suppression key included the episode's own year (InfoTagVideo.getYear(), scraped per episode — not a show-level field). For a show whose seasons aired across different calendar years, the key silently changed once the show crossed into a new year, so the suppression stopped matching and prompts came back. Now keyed on the show's canonical id (shared by every episode) instead, falling back to title alone (no year) when no canonical id is available.
  • Stale remote resume position resurrecting a locally-completed item: should_apply_resume's staleness check only ran when the local resume position was non-zero. But finishing an item locally clears its resume position to 0 while still updating lastplayed — exactly the case the check needed to cover. A remote in-progress position from before that local completion could skip the check entirely and get applied, turning a finished item back into "in progress."

kodi-addon-checker and the full unit suite (111 tests) pass clean against the updated tree.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59bec2eccd

ℹ️ 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 thread script.punchplay/resources/lib/library_events.py Outdated
@PunchPlay

Copy link
Copy Markdown
Author

Updated with a round of reliability fixes, most notably the actual root cause of the "keeps disconnecting" reports we'd been chasing:

  • Token refresh was being rejected outright by the backend's network edge. The refresh call built its own bare headers and skipped the User-Agent every other request sends — a bare Python-urllib/x.x is a common bot-mitigation signature, so refresh returned 403 on every single attempt regardless of the refresh logic itself. This forced a full manual relink every time the hour-long access token expired, for every user. Confirmed fixed against a real overnight-expired token.
  • Quitting Kodi mid-playback now sends the stop event instead of silently dropping it — previously the item was left stuck "now playing" on the backend indefinitely. Confirmed fixed live.
  • Un-watching an item within the same debounce window as marking it watched no longer uploads the watch that was just undone.
  • "Never for this show" rating suppression is now keyed on the show's canonical id instead of its per-episode year (which Kodi scrapes per episode and can differ across seasons), so it no longer silently stops matching once a show crosses into a new year.
  • A stale remote resume position can no longer resurrect a resume marker on an item already finished locally — the staleness check was skipped whenever the local resume position was 0, which is exactly what finishing an item sets it to.

113 unit tests passing, kodi-addon-checker --branch omega clean.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09bc6f7794

ℹ️ 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 thread script.punchplay/resources/lib/player.py
Comment thread script.punchplay/resources/lib/player.py Outdated
@PunchPlay

Copy link
Copy Markdown
Author

Two more fixes from this round, both closing out Codex findings on the previous revision (replied inline on each):

  • A queued offline event could replay after it was already cleaned up. The service's periodic offline-queue flush snapshots pending rows before replaying them; a concurrent stop event clearing a queued event for the same session in between could still get resent from the flush's stale snapshot, resurrecting stale "in progress" state on the backend right after the authoritative stop landed. The flush now re-checks each row still exists immediately before replaying it.
  • "Never for this show" suppression was still episode-scoped. The previous round's fix swapped the unstable per-episode year for a "canonical id" — but that id (punchplay_id/tmdb_id/tvdb_id/imdb_id) turns out to come from Kodi's per-episode uniqueid too, so it carried the exact same instability under a different name; the suppression was still only ever matching the one episode it was set on. There's no show-level id in this metadata at all, so the key is now built from the show's title alone — the one field Kodi reports consistently across every episode of a show.

116 unit tests passing (3 new, 2 fixed to use realistic per-episode-varying ids instead of an unrealistic same-id-across-episodes fixture that had masked the second bug), kodi-addon-checker --branch omega clean.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 486fb92ce6

ℹ️ 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 thread script.punchplay/resources/lib/player.py Outdated
@PunchPlay

Copy link
Copy Markdown
Author

One more fix this round, closing out the latest Codex finding (replied inline):

A stale offline event could reach the backend after a new playback's start. Starting playback dispatches immediately on the post worker, while the offline queue only drains on the service loop's independent 60-second timer — a different thread with no ordering relationship to the new start. If connectivity returns right as a new item begins playing, a stop/progress event left over from a session that ended offline could still be sitting queued, and the periodic flush could deliver it after the new start — landing as the more recent event on the backend and clobbering the now-playing state the start just set.

Starting playback now drains the offline queue first, on the same single-threaded post worker immediately ahead of the start job — both still run async off Kodi's callback thread, so this doesn't reintroduce the blocking problem the original callback-thread flush was removed for.

118 unit tests passing (2 new), kodi-addon-checker --branch omega clean.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4d4bbe081f

ℹ️ 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 thread script.punchplay/resources/lib/player.py
@PunchPlay

Copy link
Copy Markdown
Author

This round is proactive hardening rather than a reply to a specific finding here — it came out of a broad internal review of the whole offline-queue/post-worker mechanism this PR has been building up over several rounds, then independently cross-checked before landing:

  • Live watched-toggle syncs no longer block the service loop. They previously called the API client directly on the service thread — up to 15-30s blocked on a slow backend, with login/logout handling, the periodic flush trigger, and rating-prompt draining all waiting behind it. They now route through the same post worker as every other network write.
  • The pre-start offline-queue drain and the new-session start now dispatch as one atomic job. Previously two separate dispatches could be split by a full queue: the drain silently dropping (only stop events got a retry fallback) while the start still got through moments later — recreating the exact "stale event lands after the new start" race the drain exists to prevent, specifically under the load conditions where it matters most.
  • A stop event that hits a full post queue now still runs its full cleanup and rating-prompt logic, instead of only being persisted for retry and silently skipping both.
  • Offline-queue replay is now pinned to the account it was queued under. A flush spanning a logout immediately followed by logging into a different account could otherwise send an old account's queued event under the new account's credentials.
  • Suppressions written under an earlier version's key format now migrate automatically on upgrade instead of silently losing effect.
  • Two smaller fixes: malformed Kodi watch-date values are now logged instead of silently discarded, and the live-sync echo-suppression tracker no longer grows unbounded for accounts with pull sync on but live toggle sync off.

125 unit tests passing (17 new), kodi-addon-checker --branch omega clean.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 161dee8cc1

ℹ️ 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 thread script.punchplay/resources/lib/player.py Outdated
Comment thread script.punchplay/resources/lib/service.py
@PunchPlay

Copy link
Copy Markdown
Author

Two more fixes this round, continuing to harden the offline-queue mechanism:

  • A playback event could reach the backend ahead of an earlier event from the same session still stuck in the offline queue. Each dispatch path (start, generic post, stop) only tracked whether its own attempt succeeded or failed — nothing stopped a later event for a session from going out live while an earlier one for that same session sat durable, arriving out of order on the backend. A session is now marked deferred the moment any of its events goes durable, and every later event for that session stays durable too until a complete flush replays the whole backlog. flush_queue() now reports back whether it actually emptied the queue (re-checking for anything a concurrent queue-full fallback added mid-replay), so callers requiring the ordering barrier never mistake a partial drain for a complete one.
  • A failed full pull sync (a manual/scan-triggered run without a since filter) could silently fall back to the previous incremental checkpoint on the next automatic run, permanently missing whatever the failed run was meant to catch up on. The checkpoint now clears on a full-pull failure so the next automatic run is full too.

131 unit tests passing (6 new), kodi-addon-checker --branch omega clean.

@PunchPlay

Copy link
Copy Markdown
Author

@greptileai review

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.

1 participant