PCIOS-176: Time left display does not factor in deselected chapters. - #4771
PCIOS-176: Time left display does not factor in deselected chapters.#4771pocketcasts wants to merge 1 commit into
Conversation
|
Claude finished @pocketcasts's task in 3m 4s —— View job PR Review: Time left factoring in deselected chapters
Nice, focused change. The core 🔴 Bug: chapter-scoped call site gets an episode-wide subtraction
🟠 Reference-point mismatch while scrubbing
func remainingDeselectedDuration(after time: TimeInterval? = nil) -> TimeInterval {
chapterManager.deselectedDuration(after: time ?? currentTime())
}then call 🟡 Minor
SummaryThe approach and the core computation are sound, but the chapter-duration call path (finding #1) looks like a genuine display regression and the scrubbing reference-point issue (#2) is worth fixing together — both stem from Detailed notes left as inline comments. |
Generated by 🚫 Danger |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| func updateUpTo(upTo: TimeInterval, duration: TimeInterval, moveSlider: Bool) { | ||
| let remaining = max(0, duration - upTo) | ||
| let remaining = max(0, duration - upTo - PlaybackManager.shared.remainingDeselectedDuration()) |
There was a problem hiding this comment.
Reference-point mismatch during scrubbing. remainingDeselectedDuration() is hard-coded to measure deselected chapters after PlaybackManager.shared.currentTime(), but this method computes remaining relative to the upTo argument. On the provisional-scrub path (sliderDidProvisionallySlide(to:) → updateUpTo(upTo: time, …)), upTo is the drag position while currentTime() is still the real playhead, so the subtracted amount doesn't match the position being displayed. As the user scrubs across deselected chapters the label won't track correctly.
Consider threading the position through instead of hard-coding currentTime():
// PlaybackManager
func remainingDeselectedDuration(after time: TimeInterval? = nil) -> TimeInterval {
chapterManager.deselectedDuration(after: time ?? currentTime())
}and call remainingDeselectedDuration(after: upTo) here.
| func deselectedDuration(after time: TimeInterval) -> TimeInterval { | ||
| visibleChapters | ||
| .filter { !$0.isPlayable() } | ||
| .reduce(0) { total, chapter in | ||
| let chapterEnd = chapter.startTime.seconds + chapter.duration | ||
| if chapterEnd <= time { | ||
| return total | ||
| } else if chapter.startTime.seconds >= time { | ||
| return total + chapter.duration | ||
| } else { | ||
| return total + (chapterEnd - time) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic here is correct and readable. Two notes:
- No test coverage. There's an existing
ChapterManagerTestssuite and this is pure, easily-testable logic. Worth adding cases for the three branches: chapter fully beforetime(excluded), fully aftertime(full duration counted), and straddlingtime(partialchapterEnd - timecounted). - Uses
startTime.seconds/durationrather thaneffectiveStartTime. For generated chapters shifted by dynamic ads (resolvedPlaybackStartTime), this measures on the reference timeline. It's consistent withchaptersForTimeabove, so likely fine, but flagging in case deselected generated chapters need the resolved position.
| var totalDuration = PlaybackManager.shared.queue.upNextTotalDuration(includePlayingEpisode: false) | ||
| if let episode = PlaybackManager.shared.currentEpisode() { | ||
| totalDuration += episode.duration.seconds - PlaybackManager.shared.currentTime() | ||
| totalDuration += episode.duration.seconds - PlaybackManager.shared.currentTime() - PlaybackManager.shared.remainingDeselectedDuration() |
There was a problem hiding this comment.
Minor: unlike the other four call sites, this one isn't wrapped in max(0, …). If the remaining deselected duration exceeds the current episode's remaining time, this term goes negative and slightly under-counts the Up Next total. Low impact since it's summed with the rest of the queue, but worth clamping for consistency.
2 participants
Resolves https://linear.app/a8c/issue/PCIOS-176/time-left-display-does-not-factor-in-deselected-chapters
Summary
The time left display for the currently playing episode now subtracts the duration of deselected (skipped) chapters, so the remaining time reflects only the content that will actually be played.
Changes
Verification
Confidence
MEDIUM - The logic is straightforward and compiles without errors. The fix adjusts the currently playing episode time remaining only (chapter data is only loaded for the active episode).
Known Issues
This PR was created autonomously by linear-solver.
Triage complexity: medium | Linear issue