release: prepare v8.0.0#90
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR upgrades SpeakSwiftly to 9.0.0 and TextForSpeech to 0.22.0, adds route-appropriate request-purpose defaults (live → .speech; files/batches → .audioFile), propagates an optional prefacePolicy through request context defaults, and updates MCP descriptions, tests, and release notes. ChangesRequest Purpose Feature with Dependency Upgrade
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/SpeakSwiftlyServer/Host/ServerModels.swift (1)
48-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard currently drops purpose-only or preface-only request contexts.
makeSpeechRequestContextstill returnsnilunless legacy fields are present. That discards meaningful new context when onlyreqPurpose(e.g..audioFile) and/orprefacePolicyis set.💡 Proposed fix
guard merged.source != nil || merged.topic != nil || merged.cwd != nil || merged.repoRoot != nil || !merged.attributes.isEmpty + || merged.prefacePolicy != nil + || merged.reqPurpose != .speech else { return nil }🤖 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/ServerModels.swift` around lines 48 - 66, The guard in makeSpeechRequestContext (building SpeakSwiftly.RequestContext) currently returns nil unless legacy fields are present, dropping contexts that only set reqPurpose or prefacePolicy; update the guard to also consider merged.reqPurpose (non-nil) and merged.prefacePolicy (non-default/non-nil as appropriate) so requests with only purpose or prefacePolicy are accepted—modify the guard condition that checks merged.source/topic/cwd/repoRoot/attributes to include checks for merged.reqPurpose and merged.prefacePolicy on the SpeakSwiftly.RequestContext produced in this function.
🤖 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.
Outside diff comments:
In `@Sources/SpeakSwiftlyServer/Host/ServerModels.swift`:
- Around line 48-66: The guard in makeSpeechRequestContext (building
SpeakSwiftly.RequestContext) currently returns nil unless legacy fields are
present, dropping contexts that only set reqPurpose or prefacePolicy; update the
guard to also consider merged.reqPurpose (non-nil) and merged.prefacePolicy
(non-default/non-nil as appropriate) so requests with only purpose or
prefacePolicy are accepted—modify the guard condition that checks
merged.source/topic/cwd/repoRoot/attributes to include checks for
merged.reqPurpose and merged.prefacePolicy on the SpeakSwiftly.RequestContext
produced in this function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4fee6e89-7e53-4e66-b243-843f7f28efb6
📒 Files selected for processing (11)
Package.resolvedPackage.swiftSources/SpeakSwiftlyServer/HTTP/HTTPSpeechRoutes.swiftSources/SpeakSwiftlyServer/Host/ServerModels.swiftSources/SpeakSwiftlyServer/MCP/MCPClientIdentity.swiftSources/SpeakSwiftlyServer/MCP/MCPResources.swiftSources/SpeakSwiftlyServer/MCP/MCPToolCatalog.swiftTests/SpeakSwiftlyServerLibraryTests/HTTPWorkflowTests.swiftTests/SpeakSwiftlyServerLibraryTests/HostStateTests.swiftTests/SpeakSwiftlyServerLibraryTests/MCPCatalogRuntimeTests.swiftdocs/releases/v8.0.0-release-notes.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 498fd2dda9
ℹ️ 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".
| defaults: SpeechRequestContextDefaults = .init(), | ||
| ) -> SpeakSwiftly.RequestContext? { | ||
| let merged = SpeakSwiftly.RequestContext( | ||
| reqPurpose: defaults.reqPurpose, |
There was a problem hiding this comment.
Preserve caller-supplied request purpose when merging context
makeSpeechRequestContext now always sets reqPurpose from route/tool defaults, so any request_context.reqPurpose provided by HTTP or MCP callers is silently ignored. This breaks the new contract described in this release (callers can provide the shared RequestContext shape, including reqPurpose) and prevents valid overrides such as audioStream, causing downstream requests to be misclassified even when clients explicitly set a purpose.
Useful? React with 👍 / 👎.
Release
api/request-purpose-preface-policymainupdates behind pull request review and CIv8.0.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
Chores
New Features
Documentation