release: prepare v6.2.0#87
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR integrates real-time playback event streaming from the SpeakSwiftly runtime into the server host, extends core data models to carry playback event snapshots through control responses and job progress tracking, exposes playback state via new MCP resources, and broadens support for additional speech backend variants. ChangesPlayback Event Streaming Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/maintainers/speakswiftly-api-coverage-matrix.md (1)
72-73: ⚡ Quick winConsolidate duplicated
runtime.playback.snapshot()entries to reduce drift risk.Line 72 and Line 73 split one symbol into two rows. Consider merging into a single row with both read endpoints/resources in Notes to avoid future partial updates.
🤖 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 `@docs/maintainers/speakswiftly-api-coverage-matrix.md` around lines 72 - 73, The table currently has duplicate rows for the same symbol runtime.playback.snapshot(); merge them into a single row so the symbol appears once and include both read endpoints (GET /playback/queue and GET /playback/state) and all referenced resources (speak-swiftly://overview, speak-swiftly://playback/queue, speak-swiftly://playback) in the Notes column, combining the two notes (playback queue read model and playback state reads/control settling) into one concise note to prevent future drift.Sources/SpeakSwiftlyServer/Host/ServerHost+RuntimeLifecycle.swift (1)
19-74: Run SwiftPM validation on the selected Xcode toolchain before release tagging.Please use:
xcrun swift buildxcrun swift testAs per coding guidelines, "Use
xcrun swift buildandxcrun swift testas the default first-pass validation commands so repo-local SwiftPM work stays on the Xcode-selected toolchain".🤖 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 `@Sources/SpeakSwiftlyServer/Host/ServerHost`+RuntimeLifecycle.swift around lines 19 - 74, Add a pre-release SwiftPM validation step that runs the Xcode-selected toolchain builds/tests using `xcrun swift build` and `xcrun swift test` before finalizing release tagging; locate the release-finalization or lifecycle completion path (e.g., near ServerHost+RuntimeLifecycle.swift functions start() or shutdown(), or wherever release/tagging is invoked) and invoke those two commands (capturing and failing on non-zero exit) so the process aborts on build or test failures.Sources/SpeakSwiftlyServer/Host/ServerHost+EventMapping.swift (1)
149-213: ⚡ Quick winExtract playback event mapping into a dedicated
ServerHostextension file.Please move
mapPlaybackEvent(_:)andPlaybackEventBaseinto a focused file (for example,ServerHost+PlaybackEventMapping.swift) to keep this extension from accumulating mixed responsibilities.As per coding guidelines,
**/*.swift: "Keep source files small and role-focused; split shared support into explicit helper or extension files instead of growing mixed-responsibility entry points".Also applies to: 373-409
🤖 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 `@Sources/SpeakSwiftlyServer/Host/ServerHost`+EventMapping.swift around lines 149 - 213, Move the playback-mapping logic out of the large EventMapping file into a focused extension file named something like ServerHost+PlaybackEventMapping.swift: create a new file that imports the same modules and defines an extension ServerHost containing the mapPlaybackEvent(_:) function and the PlaybackEventBase type (keeping the same access levels), ensuring it references TimestampFormatter, PlaybackEventSnapshot, ActiveRequestSnapshot and QueuedRequestSnapshot exactly as before; then remove the moved definitions from ServerHost+EventMapping.swift so there are no duplicate symbols and build visibility remains unchanged.
🤖 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 `@Tests/SpeakSwiftlyServerLibraryTests/HostStateTests.swift`:
- Around line 152-162: The test reads host.jobSnapshot once immediately after
awaiting playback.latestEvent which can miss a concurrently-appended progress
entry; change the assertion to poll/refetch jobSnapshot (call
host.jobSnapshot(id: jobID) in a short-loop with a timeout) and rebuild
playbackProgress from snapshot.history each iteration until the predicate on
playbackProgress (matching stage "playback_preroll_ready",
playbackEvent?.requestID == jobID and bufferedAudioMS == 240) succeeds or the
timeout elapses; reference the existing symbols playback.latestEvent,
host.jobSnapshot(id:), snapshot.history, playbackProgress and
ServerProgressEvent when implementing the retry/wait loop to stabilize the
assertion.
---
Nitpick comments:
In `@docs/maintainers/speakswiftly-api-coverage-matrix.md`:
- Around line 72-73: The table currently has duplicate rows for the same symbol
runtime.playback.snapshot(); merge them into a single row so the symbol appears
once and include both read endpoints (GET /playback/queue and GET
/playback/state) and all referenced resources (speak-swiftly://overview,
speak-swiftly://playback/queue, speak-swiftly://playback) in the Notes column,
combining the two notes (playback queue read model and playback state
reads/control settling) into one concise note to prevent future drift.
In `@Sources/SpeakSwiftlyServer/Host/ServerHost`+EventMapping.swift:
- Around line 149-213: Move the playback-mapping logic out of the large
EventMapping file into a focused extension file named something like
ServerHost+PlaybackEventMapping.swift: create a new file that imports the same
modules and defines an extension ServerHost containing the mapPlaybackEvent(_:)
function and the PlaybackEventBase type (keeping the same access levels),
ensuring it references TimestampFormatter, PlaybackEventSnapshot,
ActiveRequestSnapshot and QueuedRequestSnapshot exactly as before; then remove
the moved definitions from ServerHost+EventMapping.swift so there are no
duplicate symbols and build visibility remains unchanged.
In `@Sources/SpeakSwiftlyServer/Host/ServerHost`+RuntimeLifecycle.swift:
- Around line 19-74: Add a pre-release SwiftPM validation step that runs the
Xcode-selected toolchain builds/tests using `xcrun swift build` and `xcrun swift
test` before finalizing release tagging; locate the release-finalization or
lifecycle completion path (e.g., near ServerHost+RuntimeLifecycle.swift
functions start() or shutdown(), or wherever release/tagging is invoked) and
invoke those two commands (capturing and failing on non-zero exit) so the
process aborts on build or test failures.
🪄 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: 0bcdc837-c2f8-4e62-9aad-93082ec0b2ee
📒 Files selected for processing (31)
.codex-plugin/plugin.jsonAPI.mdPackage.resolvedPackage.swiftROADMAP.mdSources/SpeakSwiftlyServer/Config/RuntimeStartupConfiguration.swiftSources/SpeakSwiftlyServer/EmbeddedServerSession.swiftSources/SpeakSwiftlyServer/Host/EmbeddedServerSnapshots.swiftSources/SpeakSwiftlyServer/Host/RequestEventModels.swiftSources/SpeakSwiftlyServer/Host/ServerHost+ControlResponses.swiftSources/SpeakSwiftlyServer/Host/ServerHost+EventMapping.swiftSources/SpeakSwiftlyServer/Host/ServerHost+RequestEvents.swiftSources/SpeakSwiftlyServer/Host/ServerHost+RuntimeLifecycle.swiftSources/SpeakSwiftlyServer/Host/ServerHost+Snapshots.swiftSources/SpeakSwiftlyServer/Host/ServerHost.swiftSources/SpeakSwiftlyServer/Host/SpeakSwiftlyRuntimeAdapter.swiftSources/SpeakSwiftlyServer/Host/SpeakSwiftlyRuntimeServing.swiftSources/SpeakSwiftlyServer/MCP/MCPPrompts.swiftSources/SpeakSwiftlyServer/MCP/MCPResources.swiftSources/SpeakSwiftlyServer/MCP/MCPSubscriptionBroker.swiftTests/SpeakSwiftlyServerLibraryTests/HTTPWorkflowTests.swiftTests/SpeakSwiftlyServerLibraryTests/HostStateTests.swiftTests/SpeakSwiftlyServerLibraryTests/MCPCatalogListingTests.swiftTests/SpeakSwiftlyServerLibraryTests/MCPCatalogResourceTests.swiftTests/SpeakSwiftlyServerLibraryTests/MCPCatalogRuntimeTests.swiftTests/SpeakSwiftlyServerLibraryTests/MockRuntime+TestControl.swiftTests/SpeakSwiftlyServerLibraryTests/MockRuntime.swiftdocs/codex-hooks-tts.mddocs/maintainers/speakswiftly-api-coverage-matrix.mdhooks/hooks.jsonskills/speak-swiftly-codex-hooks/SKILL.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbef03c1a4
ℹ️ 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".
| generationQueueStatus = queueStatusSnapshot(from: generationSnapshot) | ||
| playbackQueueStatus = queueStatusSnapshot(from: playbackSnapshot) | ||
| playbackStatus = PlaybackStatusSnapshot(summary: playbackSnapshot) | ||
| playbackStatus = PlaybackStatusSnapshot(summary: playbackSnapshot, latestEvent: playbackStatus.latestEvent) |
There was a problem hiding this comment.
Avoid treating snapshot capture time as playback change
applyRuntimeStateSnapshots() now rebuilds playbackStatus from every runtime.playbackSnapshot() call, and PlaybackStatusSnapshot(summary:) includes volatile fields like updatedAt (from capturedAt) and sequence. In refreshRuntimeDerivedState(), playback notifications are emitted when playbackStatus != previousPlaybackStatus, so unchanged playback state can still look "changed" on routine refreshes (for example, during non-playback job progress), generating spurious .playbackChanged events and extra MCP playback/playback/queue notifications. This should compare semantic playback fields (or preserve prior volatile fields when state is unchanged) before emitting change events.
Useful? React with 👍 / 👎.
Release
runtime/playback-event-updatesmainupdates behind pull request review and CIv6.2.0will 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
Chores