release: prepare v7.2.6#76
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis pull request expands the public PlaybackEvent vocabulary, adds lifecycle hooks and event-aware publishing APIs, wires event emission throughout the playback queue and runtime, updates tests and test helpers to assert milestone semantics, and updates documentation and CI docs. ChangesTyped Playback Event System
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related Issues
Suggested Labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Tests/SpeakSwiftlyTests/Support/TestSupport.swift (1)
321-514:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
PlaybackSpy.playcan leaveplaybackStatestuck at.playingon error/cancelLine 509 can throw before the Line 512 reset runs, so
state()may report stale.playingafter failed playback. Move the reset intodeferright after setting.playing.Suggested fix
play: { [self] _, text, tuningProfile, stream, onEvent in lock.withLock { playCount += 1 playbackState = .playing } + defer { + lock.withLock { + playbackState = .idle + } + } let thresholds = PlaybackThresholdController(text: text, tuningProfile: tuningProfile).thresholds @@ - lock.withLock { - playbackState = .idle - } - return PlaybackSummary(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/SpeakSwiftlyTests/Support/TestSupport.swift` around lines 321 - 514, PlaybackSpy.play can throw/cancel after setting playbackState = .playing and before it is reset back to .idle, leaving state() stuck at .playing; move the reset into a defer immediately after you set playbackState = .playing so it's always executed (wrap the existing lock.withLock { playbackState = .playing } with a defer that resets playbackState = .idle via lock.withLock), ensuring the rest of the logic in play (the async chunk loop, behavior switch, and possible throws) still runs but the state is reliably restored on exit or error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/SpeakSwiftly/Runtime/WorkerRuntime`+ControlRequests.swift:
- Around line 380-385: cancelRequest is unconditionally calling
publishPlaybackUpdate(.queueChanged(...)) which emits false-positive playback
events when only generation-side state changes; update cancelRequest to take the
current playback snapshot (via the same snapshot type used by
publishPlaybackUpdate) before performing cancellation, perform the
cancellation/mutation, then compute the new playback snapshot and only call
publishPlaybackUpdate(eventFromSnapshot: { .queueChanged(...) }) if the
activeRequest or queuedRequests differ (compare snapshot.activeRequest and
snapshot.queuedRequests with !=) so that queueChanged is emitted only when the
playback queue truly changed.
---
Outside diff comments:
In `@Tests/SpeakSwiftlyTests/Support/TestSupport.swift`:
- Around line 321-514: PlaybackSpy.play can throw/cancel after setting
playbackState = .playing and before it is reset back to .idle, leaving state()
stuck at .playing; move the reset into a defer immediately after you set
playbackState = .playing so it's always executed (wrap the existing
lock.withLock { playbackState = .playing } with a defer that resets
playbackState = .idle via lock.withLock), ensuring the rest of the logic in play
(the async chunk loop, behavior switch, and possible throws) still runs but the
state is reliably restored on exit or error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a8f3918-2cf7-4625-badc-4a5ae6d0364b
📒 Files selected for processing (14)
Sources/SpeakSwiftly/API/PlaybackObservation.swiftSources/SpeakSwiftly/Playback/PlaybackHooks.swiftSources/SpeakSwiftly/Playback/PlaybackQueue+Execution.swiftSources/SpeakSwiftly/Playback/RuntimePlaybackEvents.swiftSources/SpeakSwiftly/Playback/RuntimePlaybackQueueCommands.swiftSources/SpeakSwiftly/Runtime/WorkerRuntime+ControlRequests.swiftSources/SpeakSwiftly/Runtime/WorkerRuntime+RequestObservation.swiftSources/SpeakSwiftly/Runtime/WorkerRuntimeLifecycle.swiftSources/SpeakSwiftly/SpeakSwiftly.docc/RuntimeQuickStart.mdSources/SpeakSwiftly/SpeakSwiftly.docc/SpeakSwiftly.mdTests/SpeakSwiftlyTests/API/LibrarySurfaceTests.swiftTests/SpeakSwiftlyTests/Runtime/WorkerRuntimeControlSurfaceTests.swiftTests/SpeakSwiftlyTests/Support/TestSupport.swiftdocs/maintainers/typed-observation-api.md
💤 Files with no reviewable changes (1)
- Sources/SpeakSwiftly/Playback/RuntimePlaybackQueueCommands.swift
![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: c5e47c0976
ℹ️ 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".
| let latestUpdate = if let latestPlaybackUpdate = playbackObservationBroker.latestUpdate { | ||
| latestPlaybackUpdate | ||
| } else { |
There was a problem hiding this comment.
playbackUpdates() replays playbackObservationBroker.latestUpdate to every new subscriber. After this commit, updates can carry one-shot milestones like .started, .firstChunk, or .completed, so a subscriber that attaches later can receive a stale milestone and treat it as a fresh live event (for example, re-running completion/start handlers on subscribe). For milestone-style events, the initial emission should be synthesized from the current snapshot (typically .stateChanged(snapshot.state)) instead of replaying the last arbitrary event.
Useful? React with 👍 / 👎.
| await publishPlaybackUpdate(eventFromSnapshot: { snapshot in | ||
| .queueChanged( | ||
| activeRequest: snapshot.activeRequest, | ||
| queuedRequests: snapshot.queuedRequests, | ||
| ) |
There was a problem hiding this comment.
In the cancelRequest control path, playback observers are only sent .queueChanged(...). When the cancelled request is the active playback and no successor starts immediately, the active request transitions to nil without an .activeRequestChanged(nil) milestone, so consumers that key off the dedicated active-request event can miss this transition. Emit an activeRequestChanged update here (or alongside this queue update) to keep milestone coverage consistent for cancellations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79305454bf
ℹ️ 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".
| await publishPlaybackUpdate( | ||
| event: .outputDeviceChanged(previousDevice: previousDevice, currentDevice: currentDevice), | ||
| ) |
There was a problem hiding this comment.
When an output route changes during an active playback request, AudioPlaybackDriver.handleObservedOutputDeviceChange emits .outputDeviceChanged to both environmentEventSink and activeEventSink (Sources/SpeakSwiftly/Playback/AudioPlaybackDriver+EnvironmentRecovery.swift lines 38-51). This commit now publishes a public playback update in both handlers (handlePlaybackEvent and handlePlaybackEnvironmentEvent in RuntimePlaybackEvents.swift), so subscribers can receive the same PlaybackEvent.outputDeviceChanged twice for one hardware change and run side effects twice (for example, duplicate UI/device-change handling). Consider publishing from only one path or adding deduplication.
Useful? React with 👍 / 👎.
1 participant
Release
plan/issue-45mainupdates behind pull request review and CIv7.2.6will be created after CI and the review-comment gate pass, so failed or still-discussed release candidates do not get taggedReview Loop
Before merge and tagging,
scripts/repo-maintenance/release.shwatches CI and stops on review comments unless the maintainer has already addressed or resolved them and reruns with--review-comments-addressed.Summary by CodeRabbit
New Features
Documentation
Tests
Chores