feat(player): add gapless queue handoff and SPA navigation - #248
feat(player): add gapless queue handoff and SPA navigation#248tui2019 wants to merge 72 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds tighter WebView/PlayerService coordination to improve playback continuity: native YouTube Music queue injection for gapless transitions, router-based navigation to reduce full reloads, and startup autoplay suppression/restoration handling.
Changes:
- Inject the expected next track into YouTube Music’s native “Up Next” queue and advance locally when YTM auto-advances.
- Add startup autoplay blocking knobs and restoration flow that waits for a server-restored track before falling back.
- Prefer in-page router navigation and preload the YTM home shell once logged in.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/Kaset/Views/SingletonPlayerWebView+QueueInjection.swift | Adds JS-based “Play next” injection via DOM + JSON.stringify interception. |
| Sources/Kaset/Views/SingletonPlayerWebView+PlaybackControls.swift | Adds a JS toggle to block/unblock autoplay at runtime. |
| Sources/Kaset/Views/SingletonPlayerWebView+ObserverScript.swift | Honors the autoplay-block flag in playback observers. |
| Sources/Kaset/Views/MiniPlayerWebView.swift | Adds home preload, router navigation, updated autoplay bootstrap flags. |
| Sources/Kaset/Views/MiniPlayerViews.swift | Allows the persistent player view to exist without an immediate videoId. |
| Sources/Kaset/Views/MainWindow.swift | Keeps the persistent web view alive while logged in (even without pending video). |
| Sources/Kaset/Services/Player/PlayerService.swift | Tracks restoration wait state and last injected web-queue videoId. |
| Sources/Kaset/Services/Player/PlayerService+WebQueueSync.swift | Implements web-queue sync/injection and restoration metadata application. |
| Sources/Kaset/Services/Player/PlayerService+Queue.swift | Re-syncs web queue when persisting queue/session changes. |
| Sources/Kaset/Services/Player/PlayerService+PlaybackRestoration.swift | Defers restored playback until server-restored track is observed (or times out). |
| Sources/Kaset/Services/Player/PlayerService+PlaybackControls.swift | Syncs web queue after playback start; changes resume/next/previous autoplay behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com> # Conflicts: # Sources/Kaset/Services/Player/PlayerService+PlaybackControls.swift # Sources/Kaset/Services/Player/PlayerService+Queue.swift # Sources/Kaset/Views/MiniPlayerWebView.swift # Sources/Kaset/Views/SingletonPlayerWebView+PlaybackControls.swift
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
db07ab2 to
1e4f415
Compare
1e4f415 to
695f39c
Compare
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
695f39c to
f49ee0b
Compare
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
2a10062 to
b2f23e3
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
An unresolved media identity lasting beyond the two short retries can permanently discard the track-ended event and leave playback stuck.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
docs/adr/0032-gapless-playback-native-queue.md:62
- The fallback is not always three seconds: while an advertisement is active,
transitionFallbackRetryDelayextends the deadline by up to 15 seconds (18 seconds total, as asserted byWebPlaybackTransitionFallbackPolicyTests). Document the bounded advertisement grace period here so the ADR matches the implemented behavior.
Tests/KasetTests/Helpers/MusicPlaybackObserverTestContext.swift:8 - This newly extracted helper retains a force unwrap, which violates the repository's explicit no-force-unwrap policy. Make
make()throwing and use#require(JSContext()), then update its test callers to usetry.
let context = JSContext()!
- Files reviewed: 52/52 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
All three findings from the
Final local verification on this head: strict lint clean, @codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
🟡 Not ready to approve
Media identity can change without advancing its generation, causing valid native handoffs to be rejected and fall back after timeout.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
Tests/KasetTests/Helpers/MusicPlaybackObserverTestContext.swift:8
- This newly added force unwrap violates the repository's no-force-unwrap rule. Handle the optional
JSContextexplicitly so a creation failure produces a clear test failure instead of an unconditional trap.
let context = JSContext()!
Sources/Kaset/Views/SingletonPlayerWebView+ObserverScript.swift:532
- When a source transition is first observed with the old logical ID,
mediaIdentityNeedsRefreshis set. Ifplayingthen sees the new ID before the nextsendUpdate, this branch updatesmediaVideoIdand clears that flag without advancingmediaGeneration. Swift requires a newer generation before accepting a queue-entry change, so the expected native handoff is rejected until its timeout and falls back to a full load. Treat a refreshed changed identity as a binding transition so the occurrence generation advances.
if (videoId && (!mediaVideoId || (identityChanged && mediaIdentityNeedsRefresh))) {
mediaVideoId = videoId;
video.__kasetBoundVideoId = videoId;
mediaIdentityNeedsRefresh = false;
mediaIdentityUncertain = false;
mediaIdentityTransitionFromVideoId = '';
mediaIdentityIsInitialBinding = false;
- Files reviewed: 52/52 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
The latest Copilot finding is addressed and resolved in Latest local gate: strict lint clean, @codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
🟡 Not ready to approve
Unresolved media identity can silently discard the only terminal event, leaving playback or end-of-queue suppression unhandled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
Sources/Kaset/Views/SingletonPlayerWebView+ObserverScript.swift:814
- When media identity stays unresolved through this deadline, this returns without delivering any usable terminal event. The initial
TRACK_ENDEDwas marked uncertain and is discarded byPlaybackBridgeMultiplexer, soPlayerService.handleTrackEndednever runs: a failed native auto-advance can leave playback stuck, and end-of-queue autoplay suppression is never applied. On deadline, route the occurrence through a deterministic Swift recovery (or otherwise emit a terminal signal that Swift accepts) rather than silently dropping it.
if (retryNow > identityRetryDeadline) return;
Tests/KasetTests/Helpers/MusicPlaybackObserverTestContext.swift:8
- This newly added force unwrap violates the repository's explicit no-force-unwrap policy. Handle the optional
JSContextinitializer with a guard so a context creation failure produces a clear test failure instead of an unconditional unwrap.
let context = JSContext()!
- Files reviewed: 52/52 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
Addressed the suppressed Copilot findings from the prior head in
Final local gate on this head: strict lint clean, @codex review |
There was a problem hiding this comment.
🟡 Human review recommended
The extensive asynchronous WebView, undocumented DOM, restoration, and queue-state interactions warrant final human runtime review.
Review details
- Files reviewed: 52/52 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
![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: 35d7e42938
ℹ️ 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".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
Addressed the remaining suppressed Copilot finding in When identity is still unresolved at the five-second deadline, JavaScript now emits a distinct
Poll-based identity repair remains on the pending ended occurrence, while live Final local gate: strict lint clean, @codex review |
There was a problem hiding this comment.
🟡 Human review recommended
It substantially changes timing-sensitive WebView, queue, restoration, and concurrency behavior using undocumented YouTube Music internals.
Review details
- Files reviewed: 52/52 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
The latest Codex redirect finding is addressed and resolved in Latest local gate: strict lint clean, @codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
🟡 Human review recommended
The extensive WebView, queue, restoration, and concurrency state-machine changes require final human runtime review, and one claimed synchronization hook remains unused.
Review details
Suppressed comments (1)
Sources/Kaset/Services/Audio/EqualizerService.swift:304
- This synchronization hook has no call sites, so it does not replace the equalizer persistence wait as claimed in the PR description. Please either use it from the persistence test that needs deterministic completion or remove the unused API.
func awaitPendingPersistence() async {
let task = self.persistTask
await task?.value
}
- Files reviewed: 52/52 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
Addressed the final suppressed Copilot note in Verification: strict lint clean, @codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
🟡 Human review recommended
The 53-file WebView playback state-machine change requires final human runtime validation despite strong regression coverage.
Review details
- Files reviewed: 53/53 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
trying this version of the app locally, load time between song has been greatly reduced, however I am noticing that the start of the next song gets cut off (I'd say its about half a second, small but noticeable on songs that are meant to be continuous) |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
@AnthonyLenglet do you mind testing the latest build when you get a chance? |
Description
This PR adds best-effort gapless playback by mirroring Kaset's expected next queue occurrence into YouTube Music's native Up Next queue. It also keeps the YouTube Music app shell warm and prefers the web player's SPA router before falling back to a full
watchpage load.Kaset's local queue remains the source of truth. Manual Next/Previous actions—including media keys—advance Kaset's queue through deterministic
play(song:)/loadVideo(videoId:)navigation. Natural track-end playback uses the native queue only after the expected source/target relationship has been confirmed. Kaset commits the handoff only when the media-bound observer reports the expected target; stale events, wrong tracks, queue edits, failed injection, or timeout all fall back to deterministic loading.AI Prompt (Optional)
🤖 AI Prompt Used
AI Tools: Gemini CLI for the initial implementation; Codex for follow-up diagnosis, hardening, tests, and documentation.
Type of Change
Related Issues
Fixes #241
Fixes #245
Changes Made
endedevents cannot move the queue backward or double-advance it.watchEndpointrouter navigation, and generation-guards full-load fallbacks.api-explorer queue-probefor inspecting YouTube Musicnext/radio queue response shapes.docs/playback.md, and expands the playback WebView debugging skill with the new queue/handoff components.Testing
swift buildswiftlint --strict --quietswift test --skip KasetUITests --disable-xctest --no-parallelChecklist
swiftlint --strict && swiftformat .Screenshots
Bildschirmaufnahme.2026-05-09.um.8.35.21.PM.mov
Additional Notes
This is best-effort gapless playback through YouTube Music's WebView player, not sample-perfect native audio-engine gapless playback. Real-world seamlessness still depends on YouTube Music buffering, WebKit timing, network state, and YouTube's internal player behavior. When native queue state cannot be proven correct, Kaset deliberately prioritizes queue correctness over gaplessness.