feat(ios): Add TTS support for iOS using ONNX Runtime #385
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds iOS TTS support using ONNX Runtime: build scripts and CI caching/configuration, Tauri iOS build integration with a generated Cargo config and ORT path propagation, platform-specific TTS code and state for iOS, frontend TTS playback error handling, and local development docs and helper targets. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI/CD Workflow
participant Cache as Build Cache
participant ORT as ONNX Runtime Builder
participant Config as Cargo Config Generator
participant Tauri as Tauri iOS Build
participant App as iOS App
CI->>Cache: Check ONNX Runtime cache
alt Cache miss
CI->>ORT: Clone repo & build for device/simulator
ORT->>ORT: Compile static libs & create XCFramework
ORT->>Cache: Upload artifacts
end
CI->>Config: Verify XCFramework & generate .cargo/config.toml
Config->>Config: Emit ORT_LIB_LOCATION and link-search paths
CI->>Tauri: Trigger iOS build with ORT_LIB_LOCATION
Tauri->>Tauri: Link app to ONNX Runtime
Tauri->>App: Install/register TTS commands and state
App->>App: Initialize TTS (models dir, audio session)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting Comment |
Deploying maple with
|
| Latest commit: |
a85a9f9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://64db9c7d.maple-ca8.pages.dev |
| Branch Preview URL: | https://ios-tts-working.maple-ca8.pages.dev |
Greptile SummaryThis PR successfully adds Text-to-Speech (TTS) support for iOS using ONNX Runtime 1.22.2, bringing feature parity with the desktop version. The implementation is well-architected with proper iOS-specific handling. Key changes:
iOS-specific handling:
Build infrastructure:
The implementation is production-ready with proper error handling, comprehensive documentation, and thorough iOS compatibility testing. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Frontend as Frontend (React/Tauri)
participant TTSContext as TTS Context
participant RustBackend as Rust Backend (tts.rs)
participant ONNX as ONNX Runtime
participant iOS as iOS Audio System
User->>Frontend: Click TTS button
Frontend->>TTSContext: speak(text, messageId)
TTSContext->>TTSContext: preprocessTextForTTS()
TTSContext->>RustBackend: invoke("tts_synthesize", {text})
RustBackend->>RustBackend: chunk_text() & preprocess_text()
loop For each text chunk
RustBackend->>ONNX: text_encoder.run()
RustBackend->>ONNX: duration_predictor.run()
RustBackend->>ONNX: sample_noisy_latent()
loop Denoising steps (total_step=10)
RustBackend->>ONNX: vector_estimator.run()
end
RustBackend->>ONNX: vocoder.run()
end
RustBackend->>RustBackend: wav_to_base64()
RustBackend-->>TTSContext: {audio_base64, sample_rate, duration}
TTSContext->>TTSContext: base64ToBlob()
TTSContext->>iOS: navigator.audioSession.type = "playback"
TTSContext->>iOS: navigator.mediaSession.metadata = {...}
TTSContext->>TTSContext: new AudioContext/webkitAudioContext
TTSContext->>TTSContext: audioContext.decodeAudioData()
TTSContext->>iOS: source.start(0)
iOS-->>User: Audio playback
|
Additional Comments (2)
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 230:240
Comment:
When the workflow is triggered by a push event (line 9-11), `needs.check-comment.outputs.pr-number` will be undefined because the `check-comment` job is skipped. This will cause the comment step to fail with an error like "issue_number is required". You need to add a condition to only run this step when triggered by a comment event.
```suggestion
- name: Comment on PR with success
if: success() && github.event_name == 'issue_comment'
uses: actions/github-script@v7
with:
script: |
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: ${{ needs.check-comment.outputs.pr-number }},
body: '✅ TestFlight deployment completed successfully!'
});
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 242:252
Comment:
Same issue as the success comment above - when triggered by push events, `needs.check-comment.outputs.pr-number` is undefined. Add a condition to only run when triggered by comment events.
```suggestion
- name: Comment on PR with failure
if: failure() && github.event_name == 'issue_comment'
uses: actions/github-script@v7
with:
script: |
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: ${{ needs.check-comment.outputs.pr-number }},
body: '❌ TestFlight deployment failed. Check the [workflow logs](https://github.com/' + context.repo.owner + '/' + context.repo.repo + '/actions/runs/' + context.runId + ') for details.'
});
```
How can I resolve this? If you propose a fix, please make it concise. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/testflight-on-comment.yml (1)
230-251: PR comment steps will fail on push events.When triggered by push (lines 9-11),
needs.check-comment.outputs.pr-numberis empty/undefined, causing thegithub.rest.issues.createCommentcalls to fail with an invalid issue number.🐛 Proposed fix to skip comments on push events
- name: Comment on PR with success - if: success() + if: success() && github.event_name != 'push' uses: actions/github-script@v7 with: script: | await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: ${{ needs.check-comment.outputs.pr-number }}, body: '✅ TestFlight deployment completed successfully!' }); - name: Comment on PR with failure - if: failure() + if: failure() && github.event_name != 'push' uses: actions/github-script@v7
🤖 Fix all issues with AI agents
In @.github/workflows/mobile-build.yml:
- Around line 93-108: Remove the autogenerated Cargo config block that writes to
"${WORKSPACE}/frontend/src-tauri/.cargo/config.toml" (the here-doc starting with
cat > ... EOF that creates [target.<triple>.<package>] sections), because Cargo
does not support package-specific keys in [target.<triple>] tables and those
entries will be ignored; instead delete this entire config generation and rely
on the already-set ORT_LIB_LOCATION environment variable used by the ort-sys
crate to locate the ONNX Runtime libraries.
In @frontend/src-tauri/build.rs:
- Around line 11-15: The simulator branch in build.rs computes lib_path using
the wrong directory name; update the simulator path string used when
target.contains("sim") || target.contains("x86_64") so it uses
"onnxruntime.xcframework/ios-arm64-simulator" (matching build-ios-onnxruntime.sh
and cargo config generation) instead of
"onnxruntime.xcframework/ios-arm64_x86_64-simulator"; modify the string
expression that builds lib_path (referencing lib_path, target, and ort_dir)
accordingly so simulator builds resolve and link correctly.
In @frontend/src-tauri/scripts/build-ios-onnxruntime-all.sh:
- Around line 64-67: The script calls clone_with_retry when the onnxruntime dir
is missing but does not stop if cloning fails, causing a later confusing error;
after invoking clone_with_retry (in the block that checks [ ! -d "onnxruntime"
]), verify that the onnxruntime directory now exists and if not, print a clear
error via echo and exit with a non-zero status (or propagate clone_with_retry's
failure) so the script halts immediately with a helpful message; reference the
clone_with_retry invocation and the surrounding conditional that checks for
"onnxruntime".
In @frontend/src-tauri/scripts/build-ios-onnxruntime.sh:
- Around line 196-241: The Info.plist written to "${XCFRAMEWORK_DIR}/Info.plist"
always declares an ios-arm64-simulator library even though the script may skip
simulator builds; modify the script to only emit the simulator <dict> for the
ios-arm64-simulator entry when the simulator artifact was actually built (or
when the build flag that controls simulator builds is true), e.g., detect the
simulator build flag or check for the simulator library file and conditionally
append that dictionary into the Info.plist generation logic so Xcode won't
expect a missing simulator slice.
- Line 131: The grep exclusion in the IOS_ARM64_LIBS assignment uses a basic
regex so the "|" is treated literally; update the command that builds
IOS_ARM64_LIBS (the line assigning IOS_ARM64_LIBS) to use extended regex (e.g.,
add the -E flag to grep or use egrep) or escape the alternation so that
"gtest|gmock" properly excludes both gtest and gmock archives.
In @frontend/src-tauri/scripts/setup-ios-onnxruntime.sh:
- Line 10: The default ORT_VERSION in setup-ios-onnxruntime.sh is set to 1.20.1
which mismatches other scripts/CI; update the ORT_VERSION default to 1.22.2 by
changing the line that defines ORT_VERSION
(ORT_VERSION="${ORT_VERSION:-1.20.1}") to use 1.22.2 so the shell script matches
build-ios-onnxruntime.sh and CI workflows.
🧹 Nitpick comments (8)
frontend/vite.config.ts (1)
15-19: LGTM! Configuration correctly enables iOS simulator access.The server configuration properly exposes the Vite dev server on all network interfaces, allowing iOS simulators and physical devices to connect during development. The settings are appropriate for the iOS TTS development workflow.
📝 Optional: Add explanatory comment for maintainability
Consider adding a brief comment explaining the iOS simulator requirement for future maintainers:
}, + // Bind to 0.0.0.0 to allow iOS simulators and physical devices to access the dev server server: { host: "0.0.0.0", port: 5173, strictPort: true }docs/ios-tts-local-development.md (1)
45-47: Add language specifiers to fenced code blocks.Several code blocks are missing language specifiers, which helps with syntax highlighting and screen reader accessibility. Consider adding
textor an appropriate language identifier.📝 Suggested fix
-``` +```text clang: error: version '-sim' in target triple 'arm64-apple-ios13.0-simulator-sim' is invalidSimilar changes needed at lines 91, 101, and 126.
frontend/src-tauri/scripts/build-ios-onnxruntime-all.sh (1)
106-106: Separate variable declaration from assignment to avoid masking return values.The Shellcheck warning SC2155 is valid here. If
findfails, the exit status is masked by thelocaldeclaration.🔧 Proposed fix
- local LIBS=$(find build/iOS/Release -name "*.a" -path "*Release-${SYSROOT}*" -type f | grep -v "gtest\|gmock" | sort -u) + local LIBS + LIBS=$(find build/iOS/Release -name "*.a" -path "*Release-${SYSROOT}*" -type f | grep -v "gtest\|gmock" | sort -u)frontend/src-tauri/src/tts.rs (1)
614-633: Consider adding a fallback for the HOME environment variable to match patterns elsewhere in the codebase.The iOS Documents directory approach is appropriate for persistent storage. However, the code in
proxy.rs(lines 221-223) demonstrates a better pattern by falling back toUSERPROFILEifHOMEis unavailable. WhileHOMEshould be set on iOS, applying the same defensive pattern here would improve robustness:let home = std::env::var("HOME") .or_else(|_| std::env::var("USERPROFILE")) .context("Failed to get home directory on iOS")?;This aligns with the existing error-handling approach in the codebase and ensures the function handles edge cases gracefully.
frontend/src-tauri/build.rs (1)
28-29:rerun-if-changedon a directory doesn't detect file changes within.Cargo only triggers rebuilds when the directory itself is modified (renamed, deleted), not when files inside change. Consider tracking the library file directly.
♻️ Suggested improvement
// Rerun if the onnxruntime directory changes - println!("cargo:rerun-if-changed={}", ort_dir); + println!("cargo:rerun-if-changed={}/libonnxruntime.a", lib_path);frontend/src-tauri/scripts/setup-ios-onnxruntime.sh (1)
51-68: Add--failflag to curl commands to detect HTTP errors.Without
--fail, curl will succeed even on 404 errors, silently creating empty or error-page files. Consider also adding--progress-barfor large files.♻️ Proposed improvement
echo "Downloading Info.plist..." -curl -L -o "$XCFRAMEWORK_DIR/Info.plist" \ +curl -fL -o "$XCFRAMEWORK_DIR/Info.plist" \ "$HF_BASE_URL/$ORT_VERSION/onnxruntime.xcframework/Info.plist" echo "Downloading headers..." for header in cpu_provider_factory.h onnxruntime_c_api.h onnxruntime_cxx_api.h onnxruntime_cxx_inline.h; do - curl -L -o "$XCFRAMEWORK_DIR/Headers/$header" \ + curl -fL -o "$XCFRAMEWORK_DIR/Headers/$header" \ "$HF_BASE_URL/$ORT_VERSION/onnxruntime.xcframework/Headers/$header" done echo "Downloading iOS arm64 static library (this may take a while)..." -curl -L -o "$XCFRAMEWORK_DIR/ios-arm64/libonnxruntime.a" \ +curl -fL --progress-bar -o "$XCFRAMEWORK_DIR/ios-arm64/libonnxruntime.a" \ "$HF_BASE_URL/$ORT_VERSION/onnxruntime.xcframework/ios-arm64/onnxruntime.a".github/workflows/testflight-on-comment.yml (1)
123-179: Consider extracting duplicated ONNX Runtime steps into a reusable workflow.These steps (cache, build, verify, configure) are duplicated from
mobile-build.yml. For long-term maintainability, consider a composite action or reusable workflow. Since this is temporary for testing, this can be deferred.frontend/src-tauri/src/lib.rs (1)
291-317: Consider extracting shared setup logic to reduce duplication.The iOS configuration correctly exposes TTS commands matching desktop. However, the deep link handler setup (lines 303-313) is duplicated from the Android block (lines 275-285).
While the current approach provides clarity and flexibility for platform-specific extensions, you could extract the common setup into a shared helper function to reduce maintenance burden if both platforms diverge minimally.
♻️ Optional: Extract shared deep link setup
// Shared helper function for mobile deep link setup fn setup_mobile_deep_links(app: &tauri::App) { let app_handle = app.handle().clone(); app.deep_link().on_open_url(move |event| { if let Some(url) = event.urls().first() { handle_deep_link_event(url.as_ref(), &app_handle); } }); }Then both iOS and Android blocks can call
setup_mobile_deep_links(app)in their.setup()closures.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/bun.lockis excluded by!**/*.lockfrontend/src-tauri/gen/apple/maple.xcodeproj/project.pbxprojis excluded by!**/gen/**
📒 Files selected for processing (16)
.github/workflows/mobile-build.yml.github/workflows/testflight-on-comment.ymldocs/ios-tts-local-development.mdfrontend/src-tauri/.gitignorefrontend/src-tauri/Cargo.tomlfrontend/src-tauri/build.rsfrontend/src-tauri/scripts/build-ios-onnxruntime-all.shfrontend/src-tauri/scripts/build-ios-onnxruntime.shfrontend/src-tauri/scripts/setup-ios-cargo-config.shfrontend/src-tauri/scripts/setup-ios-onnxruntime.shfrontend/src-tauri/src/lib.rsfrontend/src-tauri/src/tts.rsfrontend/src/components/UnifiedChat.tsxfrontend/src/services/tts/TTSContext.tsxfrontend/vite.config.tsjustfile
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/vite.config.tsfrontend/src/services/tts/TTSContext.tsxfrontend/src/components/UnifiedChat.tsx
🧠 Learnings (3)
📚 Learning: 2026-01-08T17:09:40.432Z
Learnt from: AnthonyRonning
Repo: OpenSecretCloud/Maple PR: 372
File: frontend/src-tauri/Cargo.toml:9-9
Timestamp: 2026-01-08T17:09:40.432Z
Learning: The OpenSecretCloud/Maple repository has comprehensive GitHub Actions workflows (e.g., desktop-build.yml) that automatically validate compilation and testing, so manual compilation verification reminders are unnecessary when reviewing version bumps or dependency updates.
Applied to files:
.github/workflows/mobile-build.yml.github/workflows/testflight-on-comment.yml
📚 Learning: 2025-03-25T19:50:07.925Z
Learnt from: AnthonyRonning
Repo: OpenSecretCloud/Maple PR: 23
File: frontend/src-tauri/src/lib.rs:14-16
Timestamp: 2025-03-25T19:50:07.925Z
Learning: In Tauri applications, updater security configurations (including endpoints, public key for verification, and dialog settings) should be defined in the tauri.conf.json file rather than duplicated in the code. When initialized with tauri_plugin_updater::Builder::new().build(), the plugin automatically reads and uses these settings from the configuration file.
Applied to files:
frontend/src-tauri/src/lib.rs
📚 Learning: 2025-03-25T19:50:07.925Z
Learnt from: AnthonyRonning
Repo: OpenSecretCloud/Maple PR: 23
File: frontend/src-tauri/src/lib.rs:14-16
Timestamp: 2025-03-25T19:50:07.925Z
Learning: In Tauri applications, the updater configuration (including security settings like server URL and public key) should be defined in tauri.conf.json rather than hardcoded in the application code. The code only needs to initialize the updater plugin with tauri_plugin_updater::Builder::new().build(), which will automatically use the settings from the configuration file.
Applied to files:
frontend/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
frontend/src/components/UnifiedChat.tsx (5)
frontend/src/services/tts/TTSContext.tsx (1)
useTTS(346-352)frontend/src/services/tts/index.ts (1)
useTTS(1-1)frontend/src/components/ui/alert.tsx (2)
Alert(49-49)AlertDescription(49-49)frontend/src/components/ui/button.tsx (1)
Button(62-62)frontend/src/components/icons/X.tsx (1)
X(3-23)
🪛 LanguageTool
docs/ios-tts-local-development.md
[style] ~44-~44: Consider an alternative verb to strengthen your wording.
Context: ...m64-sim Xcode Issue (if needed) If you see this error: ``` clang: error: version '...
(IF_YOU_HAVE_THIS_PROBLEM)
🪛 markdownlint-cli2 (0.18.1)
docs/ios-tts-local-development.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
101-101: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
126-126: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
frontend/src-tauri/scripts/build-ios-onnxruntime-all.sh
[warning] 106-106: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-ios
- GitHub Check: build-android
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-linux
- GitHub Check: Cloudflare Pages
🔇 Additional comments (19)
frontend/src-tauri/.gitignore (1)
5-11: LGTM!The gitignore patterns correctly exclude build artifacts (ONNX Runtime build directories) and generated cargo config, preventing accidental commits of machine-specific files.
docs/ios-tts-local-development.md (1)
1-166: Well-structured documentation.This guide provides comprehensive coverage of the iOS TTS local development workflow, including prerequisites, build steps, troubleshooting, and architecture notes. The content aligns well with the build scripts and CI workflows introduced in this PR.
justfile (1)
38-56: LGTM - New iOS build tooling commands.The three new justfile targets provide a clean interface for:
- Building ONNX Runtime for iOS
- Setting up cargo config after build
- Fixing the arm64-sim Xcode architecture issue
The
ios-fix-archtarget uses strict bash mode (set -euo pipefail) and includes a helpful verification hint.frontend/src-tauri/scripts/build-ios-onnxruntime-all.sh (1)
1-212: Solid build script for iOS ONNX Runtime.The script provides a complete workflow for building ONNX Runtime from source for both device and simulator targets, with proper xcframework packaging and Info.plist generation. The retry logic for cloning and the libiconv workaround are good defensive measures.
frontend/src/components/UnifiedChat.tsx (2)
2667-2685: TTS playback error UI is well-implemented.The error alert follows the existing pattern for displaying errors and provides a dismiss button for good UX. The positioning and styling are consistent with the main error alert.
One consideration: if both
errorandplaybackErrorare set simultaneously, they would overlap at the same position (top-16). This is likely rare but could be addressed with conditional positioning.
877-877: LGTM - Clean integration with TTS context.The destructuring of
playbackErrorandclearPlaybackErrorfrom theuseTTS()hook follows the established pattern and correctly integrates the new error handling capability.frontend/src-tauri/Cargo.toml (1)
56-71: Replaceortversion with the latest stable release (1.13.1).The iOS-specific dependency block is structurally correct (appropriately disables
default-featuresforort, omitsrayonfromndarray), butortversion2.0.0-rc.10is a release candidate without stability guarantees. The latest stable version is1.13.1. Either upgrade to1.13.1or document why2.0.0-rc.10is specifically required.⛔ Skipped due to learnings
Learnt from: AnthonyRonning Repo: OpenSecretCloud/Maple PR: 136 File: frontend/src-tauri/tauri.conf.json:4-4 Timestamp: 2025-07-02T22:53:39.374Z Learning: When reviewing version bump PRs, version numbers found in Cargo.lock files are typically dependency versions (like aho-corasick), not the application version. Don't suggest updating these unless they are specifically the application's own version entry..github/workflows/mobile-build.yml (2)
56-73: Good caching strategy for expensive ONNX Runtime build.The cache-then-build pattern with 90-minute timeout is appropriate for a ~5-10 minute build. The version-specific cache key (
onnxruntime-ios-built-1.22.2-v1) ensures cache invalidation when needed.
160-165: Temporary TestFlight condition is well-documented.The TODO comment clearly explains the temporary ios-tts branch condition. Remember to remove it after the PR is merged.
frontend/src-tauri/scripts/setup-ios-cargo-config.sh (2)
30-49: Well-structured config generation with validation.Good defensive checks for required artifacts before generating config. The
[env]section forORT_LIB_LOCATIONis valid Cargo config and will propagate the environment variable.Note: The
[target.*.onnxruntime]sections may not be recognized by Cargo (same concern as in CI workflow). The[env]section is the reliable mechanism.
57-63: Appropriate warning for missing simulator library.Good user experience—warns about simulator limitations without blocking device builds, which are the primary target for TestFlight deployment.
frontend/src-tauri/scripts/build-ios-onnxruntime.sh (1)
52-95: Robust retry logic for network operations.Good handling of transient network failures with 3 attempts and 10-second delays for both clone and submodule operations.
frontend/src/services/tts/TTSContext.tsx (3)
238-251: Correct iOS AudioContext handling.Good implementation of the
webkitAudioContextfallback for iOS Safari compatibility and proper handling of the suspended state that iOS requires due to autoplay policies. The error message clearly explains Lockdown Mode limitations.
46-47: Clean separation of download and playback error states.Separating
playbackErrorfrom the generalerrorstate allows appropriate UI handling—download errors are blocking while playback errors can be dismissible alerts.Also applies to: 76-76, 292-294
65-66: Correct platform gating for TTS availability.Extending TTS to iOS while explicitly excluding Android aligns with the PR objectives. The compound condition is clear and readable.
frontend/src-tauri/src/lib.rs (4)
6-8: LGTM!The conditional compilation correctly enables the TTS module for both desktop and iOS platforms while excluding Android.
263-267: LGTM!The TTS state management for iOS correctly mirrors the desktop implementation and uses the appropriate conditional compilation.
269-274: LGTM!The Android configuration correctly excludes TTS commands while maintaining existing functionality.
244-255: LGTM!The comment clarification and shared mobile builder setup are appropriate. The mutable builder pattern allows platform-specific additions (Apple Sign In for iOS, TTS state for iOS) before the final configuration.
AnthonyRonning
force-pushed
the
ios-tts-working
branch
from
5fd296c to
4f2a525
Compare
Additional Comments (2)
The comment steps should only run when triggered by Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 230:240
Comment:
When the workflow is triggered by a push event (line 9-11), the `check-comment` job is skipped (line 21 condition), which means `needs.check-comment.outputs.pr-number` is undefined. This will cause the comment step to fail with "issue_number is required".
The comment steps should only run when triggered by `issue_comment` events where `pr-number` is available.
```suggestion
- name: Comment on PR with success
if: success() && github.event_name == 'issue_comment'
uses: actions/github-script@v7
with:
script: |
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: ${{ needs.check-comment.outputs.pr-number }},
body: '✅ TestFlight deployment completed successfully!'
});
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 242:252
Comment:
Same issue as the success comment above - when triggered by push event, `needs.check-comment.outputs.pr-number` is undefined and this step will fail.
```suggestion
- name: Comment on PR with failure
if: failure() && github.event_name == 'issue_comment'
uses: actions/github-script@v7
with:
script: |
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: ${{ needs.check-comment.outputs.pr-number }},
body: '❌ TestFlight deployment failed. Check the [workflow logs](https://github.com/' + context.repo.owner + '/' + context.repo.repo + '/actions/runs/' + context.runId + ') for details.'
});
```
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/testflight-on-comment.yml (1)
230-251: PR comment steps will fail on push events due to missing pr-number.When triggered by a push event, the
check-commentjob is skipped (line 21), soneeds.check-comment.outputs.pr-numberwill be empty. This causes these comment steps to fail when trying to post to issue number${{ needs.check-comment.outputs.pr-number }}.🐛 Suggested fix - add condition to skip on push events
- name: Comment on PR with success - if: success() + if: success() && github.event_name != 'push' uses: actions/github-script@v7 with: script: | await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: ${{ needs.check-comment.outputs.pr-number }}, body: '✅ TestFlight deployment completed successfully!' }); - name: Comment on PR with failure - if: failure() + if: failure() && github.event_name != 'push' uses: actions/github-script@v7
🤖 Fix all issues with AI agents
In @frontend/src-tauri/scripts/setup-ios-cargo-config.sh:
- Around line 43-45: Remove the orphaned target configuration
`target.x86_64-apple-ios.onnxruntime` from the cargo config so it no longer
references `ios-arm64-simulator`; this must match what
`build-ios-onnxruntime-all.sh` actually produces (only `ios-arm64` and
`ios-arm64-simulator`). Alternatively, if you intend to support x86_64
simulator, add x86_64 simulator slice creation to `build-ios-onnxruntime-all.sh`
and update the target to point to the correct combined slice, but do not leave
the existing `target.x86_64-apple-ios.onnxruntime` entry pointing at
`ios-arm64-simulator` when that library isn’t built.
In @frontend/src/components/UnifiedChat.tsx:
- Around line 2667-2685: The close Button inside the TTS playback error block
(rendered when playbackError is truthy) is missing an aria-label; update the
Button element that calls clearPlaybackError to include an appropriate
aria-label (for example aria-label="Close playback error" or similar descriptive
text) so the icon-only X button is accessible to screen readers; also check
other icon-only close Buttons in this component (e.g., the general error banner)
and add consistent aria-labels where missing.
🧹 Nitpick comments (6)
frontend/vite.config.ts (1)
15-19: Makeserver.host/portconfigurable (avoid always binding to0.0.0.0)Binding to
0.0.0.0is great for iOS simulator/device access, but it also exposes the dev server to the LAN by default. Consider gating it behind an env var (and optionally making the port configurable), and please verify HMR on a physical iPhone—some setups needserver.hmr.host/clientPort.Proposed tweak (env-gated bind-all + configurable port)
-import { defineConfig } from "vite"; +import { defineConfig, loadEnv } from "vite"; import react from "@vitejs/plugin-react"; import path from "path"; import { TanStackRouterVite } from "@tanstack/router-plugin/vite"; import derPlugin from "./vite-der-plugin"; // https://vitejs.dev/config/ -export default defineConfig({ - plugins: [TanStackRouterVite(), react(), derPlugin()], - resolve: { - alias: { - "@": path.resolve(__dirname, "./src") - } - }, - server: { - host: "0.0.0.0", - port: 5173, - strictPort: true - } -}); +export default defineConfig(({ mode }) => { + const env = loadEnv(mode, process.cwd(), ""); + const bindAll = env.VITE_DEV_BIND_ALL === "true"; + const port = Number(env.VITE_DEV_PORT ?? 5173); + + return { + plugins: [TanStackRouterVite(), react(), derPlugin()], + resolve: { + alias: { + "@": path.resolve(__dirname, "./src") + } + }, + server: { + host: bindAll ? "0.0.0.0" : "localhost", + port, + strictPort: true + } + }; +});docs/ios-tts-local-development.md (1)
45-47: Add language specifiers to fenced code blocks for better rendering.Several code blocks showing error messages or directory structures lack language specifiers. Adding
textorplaintextwill improve rendering in documentation viewers and satisfy linting rules.📝 Suggested fix
-``` +```text clang: error: version '-sim' in target triple 'arm64-apple-ios13.0-simulator-sim' is invalid-``` +```text Undefined symbols for architecture arm64: "_AbslInternalSpinLockDelay_lts_20240722"-``` +```text ld: building for 'iOS-simulator', but linking in dylib built for 'iOS'-``` +```text frontend/src-tauri/ ├── onnxruntime-build/ # Build directory (can be deleted after build)Also applies to: 91-95, 101-104, 126-138
.github/workflows/mobile-build.yml (1)
87-111: Consider using the setup script instead of duplicating config generation logic.This inline cargo config generation duplicates the logic in
frontend/src-tauri/scripts/setup-ios-cargo-config.sh. Using the script would reduce duplication and ensure consistency.♻️ Suggested change
- name: Configure Cargo for iOS ONNX Runtime run: | - # Create cargo config with absolute paths for iOS builds - # This overrides ort-sys's build script to use our built-from-source library - WORKSPACE="${{ github.workspace }}" - mkdir -p "${WORKSPACE}/frontend/src-tauri/.cargo" - cat > "${WORKSPACE}/frontend/src-tauri/.cargo/config.toml" << EOF - # Auto-generated cargo config for iOS ONNX Runtime linking - # Uses absolute paths because xcodebuild may run cargo from different directories - - [target.aarch64-apple-ios.onnxruntime] - rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64"] - rustc-link-lib = ["static=onnxruntime"] - - [target.aarch64-apple-ios-sim.onnxruntime] - rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"] - rustc-link-lib = ["static=onnxruntime"] - - [target.x86_64-apple-ios.onnxruntime] - rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"] - rustc-link-lib = ["static=onnxruntime"] - EOF - - echo "Generated cargo config:" - cat "${WORKSPACE}/frontend/src-tauri/.cargo/config.toml" + chmod +x frontend/src-tauri/scripts/setup-ios-cargo-config.sh + cd frontend/src-tauri && ./scripts/setup-ios-cargo-config.sh.github/workflows/testflight-on-comment.yml (1)
154-178: Same duplication of cargo config generation as mobile-build.yml.Consider using the
setup-ios-cargo-config.shscript here as well to maintain consistency and reduce duplication across workflows.frontend/src-tauri/scripts/build-ios-onnxruntime-all.sh (1)
106-111: Declare and assign separately to avoid masking return values.The combined declaration and assignment masks the return value of
find. Iffindfails, the error won't be caught byset -e.♻️ Suggested fix
- local LIBS=$(find build/iOS/Release -name "*.a" -path "*Release-${SYSROOT}*" -type f | grep -v "gtest\|gmock" | sort -u) + local LIBS + LIBS=$(find build/iOS/Release -name "*.a" -path "*Release-${SYSROOT}*" -type f | grep -v "gtest\|gmock" | sort -u)frontend/src/services/tts/TTSContext.tsx (1)
238-252: iOS AudioContext handling is correct, but consider error message clarity.The implementation correctly handles iOS-specific requirements:
webkitAudioContextfallback for older iOS Safari- Suspended state handling (iOS requires user interaction)
- Proper error handling when AudioContext is unavailable
However, the error message on Line 243 specifically mentions "Lockdown Mode" which is iOS-specific. If
AudioContextis unavailable on other platforms (e.g., browser with restrictive permissions), this message might confuse users.💡 Consider platform-aware error message
- if (!AudioContextClass) { - throw new Error( - "Audio playback is not available. If you have Lockdown Mode enabled, TTS will not work." - ); - } + if (!AudioContextClass) { + const errorMsg = isIOS() + ? "Audio playback is not available. If you have Lockdown Mode enabled, TTS will not work." + : "Audio playback is not available. Please check your browser permissions."; + throw new Error(errorMsg); + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/src-tauri/gen/apple/maple.xcodeproj/project.pbxprojis excluded by!**/gen/**
📒 Files selected for processing (9)
.github/workflows/mobile-build.yml.github/workflows/testflight-on-comment.ymldocs/ios-tts-local-development.mdfrontend/src-tauri/scripts/build-ios-onnxruntime-all.shfrontend/src-tauri/scripts/setup-ios-cargo-config.shfrontend/src/components/UnifiedChat.tsxfrontend/src/services/tts/TTSContext.tsxfrontend/vite.config.tsjustfile
🚧 Files skipped from review as they are similar to previous changes (1)
- justfile
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/vite.config.tsfrontend/src/services/tts/TTSContext.tsxfrontend/src/components/UnifiedChat.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-08T17:09:40.432Z
Learnt from: AnthonyRonning
Repo: OpenSecretCloud/Maple PR: 372
File: frontend/src-tauri/Cargo.toml:9-9
Timestamp: 2026-01-08T17:09:40.432Z
Learning: The OpenSecretCloud/Maple repository has comprehensive GitHub Actions workflows (e.g., desktop-build.yml) that automatically validate compilation and testing, so manual compilation verification reminders are unnecessary when reviewing version bumps or dependency updates.
Applied to files:
.github/workflows/mobile-build.yml.github/workflows/testflight-on-comment.yml
🪛 LanguageTool
docs/ios-tts-local-development.md
[style] ~44-~44: Consider an alternative verb to strengthen your wording.
Context: ...m64-sim Xcode Issue (if needed) If you see this error: ``` clang: error: version '...
(IF_YOU_HAVE_THIS_PROBLEM)
🪛 markdownlint-cli2 (0.18.1)
docs/ios-tts-local-development.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
101-101: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
126-126: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
frontend/src-tauri/scripts/build-ios-onnxruntime-all.sh
[warning] 106-106: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-android
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-ios
- GitHub Check: build-linux
- GitHub Check: Cloudflare Pages
🔇 Additional comments (12)
docs/ios-tts-local-development.md (1)
1-166: Comprehensive and well-structured documentation.This guide provides clear instructions for building and testing iOS TTS locally, with useful troubleshooting sections and architecture notes. The Quick Start flow is logical and the cleanup commands help manage disk space.
frontend/src-tauri/scripts/setup-ios-cargo-config.sh (1)
1-63: Well-structured script with proper validation.The script correctly validates prerequisites, generates appropriate cargo config with absolute paths, and provides helpful warnings. The use of
set -eensures early failure on errors..github/workflows/mobile-build.yml (2)
56-73: ONNX Runtime caching and build configuration looks good.The cache key includes the version number ensuring rebuilds on version changes, and the 90-minute timeout is reasonable for building from source.
160-165: TODO comment is clear and properly scoped.The temporary condition for
ios-tts-workingbranch is well-documented with a clear TODO to remove after PR #385 merges..github/workflows/testflight-on-comment.yml (1)
6-11: Temporary workflow changes are well-documented.The push trigger and conditional logic for the
ios-tts-workingbranch are clearly marked as temporary with appropriate TODO comments.Also applies to: 20-21, 73-80
frontend/src-tauri/scripts/build-ios-onnxruntime-all.sh (2)
50-62: Nice resilience pattern with clone retry logic.The retry mechanism with exponential backoff-style delay handles transient network issues gracefully.
1-212: Well-structured build script with comprehensive error handling.The script properly validates prerequisites, handles both device and simulator builds, combines static libraries, and generates a valid xcframework with proper Info.plist. The early exit when artifacts exist is a good optimization.
frontend/src/services/tts/TTSContext.tsx (4)
46-46: LGTM: Clean separation of playback errors from status errors.The addition of
playbackErrorandclearPlaybackErrorto the context interface provides a good separation between TTS status/download errors and runtime playback errors, improving error handling granularity.Also applies to: 59-59
65-66: LGTM: Correct platform availability logic.The TTS availability check correctly enables TTS on desktop and iOS platforms while excluding Android, as intended by the PR objectives.
284-286: LGTM: Clean error state management.The error handling correctly updates
playbackErrorstate and cleans up resources viastop(). TheclearPlaybackErrorcallback provides a simple way for UI components to dismiss errors.Also applies to: 292-294
322-343: LGTM: Provider correctly exposes updated context.The context provider properly includes
playbackErrorandclearPlaybackErrorin the value object, making them available to consumers viauseTTS().frontend/src/components/UnifiedChat.tsx (1)
877-877: LGTM: Correct usage of updated TTS context.The component properly consumes the new
playbackErrorandclearPlaybackErrorfields from the TTS context.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (2)
-
.github/workflows/testflight-on-comment.yml, line 238 (link)logic: This step will fail when the workflow is triggered by push events because
needs.check-comment.outputs.pr-numberis undefined. Add condition:if: success() && github.event_name == 'issue_comment' -
.github/workflows/testflight-on-comment.yml, line 250 (link)logic: Same issue - this will fail on push events when
pr-numberis undefined. Add condition:if: failure() && github.event_name == 'issue_comment'
17 files reviewed, 6 comments
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/testflight-on-comment.yml:
- Around line 154-175: The generated custom Cargo config with non-standard
sections ([target.aarch64-apple-ios.onnxruntime] and
[target.aarch64-apple-ios-sim.onnxruntime]) is ineffective; remove the step that
creates frontend/src-tauri/.cargo/config.toml (the "Configure Cargo for iOS ONNX
Runtime" run block) or stop writing those target-specific subsections, and rely
on the existing ORT_LIB_LOCATION environment variable (already set elsewhere in
the workflow) for ort-sys linking; ensure no other steps expect the
.cargo/config.toml output or adjust them accordingly.
In `@frontend/src-tauri/src/tts.rs`:
- Around line 614-622: The code in get_tts_models_dir currently puts TTS models
into iOS Documents; change it to place the models under the app Library (either
Application Support or Caches) instead (e.g., Library/Application
Support/cloud.opensecret.maple/tts_models or
Library/Caches/cloud.opensecret.maple/tts_models) so they are not user-visible
or iCloud-backed; if you choose Application Support also set the
NSURLIsExcludedFromBackupKey attribute on the created directory (use the
appropriate iOS APIs or crates to set the exclusion flag) and update
get_tts_models_dir to create and return that Library path instead of Documents.
🧹 Nitpick comments (3)
frontend/src-tauri/scripts/setup-ios-onnxruntime.sh (2)
52-59: Add--failflag to curl commands to detect HTTP errors.Without
--fail(-f), curl returns exit code 0 even on HTTP 404/5xx responses, silently creating files with error content. This would cause confusing build failures later.♻️ Proposed fix
echo "Downloading Info.plist..." -curl -L -o "$XCFRAMEWORK_DIR/Info.plist" \ +curl -fL -o "$XCFRAMEWORK_DIR/Info.plist" \ "$HF_BASE_URL/$ORT_VERSION/onnxruntime.xcframework/Info.plist" echo "Downloading headers..." for header in cpu_provider_factory.h onnxruntime_c_api.h onnxruntime_cxx_api.h onnxruntime_cxx_inline.h; do - curl -L -o "$XCFRAMEWORK_DIR/Headers/$header" \ + curl -fL -o "$XCFRAMEWORK_DIR/Headers/$header" \ "$HF_BASE_URL/$ORT_VERSION/onnxruntime.xcframework/Headers/$header" done
61-68: Add--failflag and consider checksum verification for binary downloads.Same
--failflag issue as above. Additionally, for security-sensitive binary downloads, consider adding SHA256 checksum verification to detect tampering or corruption.♻️ Proposed fix with --fail flag
echo "Downloading iOS arm64 static library (this may take a while)..." -curl -L -o "$XCFRAMEWORK_DIR/ios-arm64/libonnxruntime.a" \ +curl -fL -o "$XCFRAMEWORK_DIR/ios-arm64/libonnxruntime.a" \ "$HF_BASE_URL/$ORT_VERSION/onnxruntime.xcframework/ios-arm64/onnxruntime.a" echo "Downloading iOS simulator static library (this may take a while)..." -curl -L -o "$XCFRAMEWORK_DIR/ios-arm64_x86_64-simulator/libonnxruntime.a" \ +curl -fL -o "$XCFRAMEWORK_DIR/ios-arm64_x86_64-simulator/libonnxruntime.a" \ "$HF_BASE_URL/$ORT_VERSION/onnxruntime.xcframework/ios-arm64_x86_64-simulator/onnxruntime.a"Note: The
onnxruntime.a→libonnxruntime.arename is correct and intentional for Unix linker conventions (-lonnxruntimeexpectslibonnxruntime.a)..github/workflows/testflight-on-comment.yml (1)
142-152: Abseil symbol check is informational only - consider failing on absence.The PR notes state that ONNX Runtime is built from source specifically because "prebuilt binaries lack required Abseil symbols." However, the
|| echofallback means the workflow won't fail if symbols are missing - it just prints a message and continues.If Abseil symbols are truly required for TTS to function, consider making this a hard failure:
Proposed fix to fail on missing Abseil symbols
- echo "Checking for Abseil symbols (should be included):" - nm ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/libonnxruntime.a 2>/dev/null | grep -i "absl" | head -20 || echo "No abseil symbols found (they may be internal)" + echo "Checking for Abseil symbols (required for TTS):" + ABSEIL_COUNT=$(nm ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/libonnxruntime.a 2>/dev/null | grep -ci "absl" || echo "0") + echo "Found ${ABSEIL_COUNT} Abseil symbol references" + if [ "${ABSEIL_COUNT}" -lt 10 ]; then + echo "::error::Expected Abseil symbols in libonnxruntime.a but found ${ABSEIL_COUNT}. Build may have failed to include dependencies." + exit 1 + fi
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/mobile-build.yml.github/workflows/testflight-on-comment.ymlfrontend/src-tauri/scripts/setup-ios-cargo-config.shfrontend/src-tauri/scripts/setup-ios-onnxruntime.shfrontend/src-tauri/src/tts.rsfrontend/src/components/UnifiedChat.tsxfrontend/vite.config.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/components/UnifiedChat.tsx
- frontend/vite.config.ts
- .github/workflows/mobile-build.yml
- frontend/src-tauri/scripts/setup-ios-cargo-config.sh
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-08T17:09:55.044Z
Learnt from: AnthonyRonning
Repo: OpenSecretCloud/Maple PR: 372
File: frontend/src-tauri/Cargo.toml:9-9
Timestamp: 2026-01-08T17:09:55.044Z
Learning: The OpenSecretCloud/Maple repository has comprehensive GitHub Actions workflows (e.g., desktop-build.yml) that automatically validate compilation and testing, so manual compilation verification reminders are unnecessary when reviewing version bumps or dependency updates.
Applied to files:
.github/workflows/testflight-on-comment.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: rust-build (i686-linux-android, i686_linux_android, I686_LINUX_ANDROID, x86, i686-linux-android24...
- GitHub Check: rust-build (armv7-linux-androideabi, armv7_linux_androideabi, ARMV7_LINUX_ANDROIDEABI, armeabi-v7...
- GitHub Check: rust-build (x86_64-linux-android, x86_64_linux_android, X86_64_LINUX_ANDROID, x86_64, x86_64-linu...
- GitHub Check: rust-build (aarch64-linux-android, aarch64_linux_android, AARCH64_LINUX_ANDROID, arm64-v8a, aarch...
- GitHub Check: Greptile Review
- GitHub Check: Cloudflare Pages
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-linux
- GitHub Check: build-ios
- GitHub Check: deploy-testflight
🔇 Additional comments (8)
frontend/src-tauri/scripts/setup-ios-onnxruntime.sh (3)
1-12: LGTM!Error handling with
set -e, path resolution, and version configuration are all correct. The defaultORT_VERSIONof1.22.2now matches the build scripts and CI workflows.
14-25: LGTM!The idempotency check with clear user instructions for re-downloading is a good UX pattern.
69-78: LGTM!Good diagnostic output showing directory contents and file sizes, which helps verify successful downloads.
.github/workflows/testflight-on-comment.yml (5)
6-21: LGTM - Well-documented temporary workaround with proper gating.The TODO comment clearly explains why the push trigger exists and when it should be removed. The condition on line 21 correctly guards against running
check-commenton push events wherecontext.payload.commentwould be undefined.
71-80: LGTM - Correct use ofalways()for dual-trigger workflow.The
always()is required here because whencheck-commentis skipped (on push events), the job would otherwise be skipped due to theneedsdependency. The ref resolution correctly handles both event types.
123-140: LGTM - Appropriate caching strategy for expensive build.The cache configuration properly separates the version-specific key from fallback restore-keys, and the 90-minute timeout is reasonable for building ONNX Runtime from source. Caching both the output and build directories helps with potential incremental rebuilds.
200-214: LGTM - Correct ORT_LIB_LOCATION for device builds.The
ORT_LIB_LOCATIONenvironment variable correctly points to theios-arm64directory for App Store builds, which target physical devices.
226-248: LGTM - Proper gating of PR comments to issue_comment events.The conditions correctly prevent attempting to post PR comments during push-triggered runs where
pr-numberoutput would be unavailable.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Additional Comments (1)
-
frontend/src-tauri/src/tts.rs, line 640 (link)style: Consider adding voice selection to the public API. Currently hardcoded to F2, but F1, M1, M2 are also available.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
16 files reviewed, 2 comments
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/testflight-on-comment.yml (1)
226-247: Success/failure comment steps will fail on push events.When the workflow is triggered by a push to
ios-tts-working, thecheck-commentjob is skipped, soneeds.check-comment.outputs.pr-numberwill be empty. This causes thegithub.rest.issues.createCommentAPI call to fail with an invalidissue_number.Add a condition to only run these steps for
issue_commentevents:🛠️ Proposed fix
- name: Comment on PR with success - if: success() + if: success() && github.event_name == 'issue_comment' uses: actions/github-script@v7 with: script: | await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: ${{ needs.check-comment.outputs.pr-number }}, body: '✅ TestFlight deployment completed successfully!' }); - name: Comment on PR with failure - if: failure() + if: failure() && github.event_name == 'issue_comment' uses: actions/github-script@v7 with: script: | await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: ${{ needs.check-comment.outputs.pr-number }}, body: '❌ TestFlight deployment failed. Check the [workflow logs](https://github.com/' + context.repo.owner + '/' + context.repo.repo + '/actions/runs/' + context.runId + ') for details.' });
♻️ Duplicate comments (1)
.github/workflows/testflight-on-comment.yml (1)
154-175: Non-standard Cargo config sections are ineffective.The
[target.aarch64-apple-ios.onnxruntime]and[target.aarch64-apple-ios-sim.onnxruntime]sections will be ignored by Cargo. TheORT_LIB_LOCATIONenvironment variable set at line 213 is the actual mechanism used byort-sysfor linking.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/testflight-on-comment.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-08T17:09:55.044Z
Learnt from: AnthonyRonning
Repo: OpenSecretCloud/Maple PR: 372
File: frontend/src-tauri/Cargo.toml:9-9
Timestamp: 2026-01-08T17:09:55.044Z
Learning: The OpenSecretCloud/Maple repository has comprehensive GitHub Actions workflows (e.g., desktop-build.yml) that automatically validate compilation and testing, so manual compilation verification reminders are unnecessary when reviewing version bumps or dependency updates.
Applied to files:
.github/workflows/testflight-on-comment.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: rust-build (aarch64-linux-android, aarch64_linux_android, AARCH64_LINUX_ANDROID, arm64-v8a, aarch...
- GitHub Check: rust-build (i686-linux-android, i686_linux_android, I686_LINUX_ANDROID, x86, i686-linux-android24...
- GitHub Check: rust-build (armv7-linux-androideabi, armv7_linux_androideabi, ARMV7_LINUX_ANDROIDEABI, armeabi-v7...
- GitHub Check: rust-build (x86_64-linux-android, x86_64_linux_android, X86_64_LINUX_ANDROID, x86_64, x86_64-linu...
- GitHub Check: Greptile Review
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-linux
- GitHub Check: build-ios
🔇 Additional comments (6)
.github/workflows/testflight-on-comment.yml (6)
6-11: Temporary push trigger is well-documented.The TODO comment clearly explains the rationale and removal timing. This is a reasonable workaround for testing workflow changes that can't be picked up from the PR branch when triggered via
issue_comment.
20-21: LGTM!The conditional correctly gates
check-commentto only run onissue_commentevents with PR context and the trigger phrase.
73-80: LGTM!The conditional and ref resolution correctly handle both trigger paths. Push events use
github.refwhile comment-triggered events use the PR head ref.
123-140: LGTM!The caching strategy is well-designed with version-specific keys and appropriate fallbacks. The 90-minute timeout is reasonable for building ONNX Runtime from source.
142-152: LGTM!Good diagnostic step to verify the ONNX Runtime build. The Abseil symbol check confirms the static linking that addresses the missing symbol issue mentioned in the PR objectives.
212-213: LGTM!The
ORT_LIB_LOCATIONenvironment variable is the correct mechanism forort-systo locate the ONNX Runtime library during the build.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Add iOS-specific ort dependencies and build configuration - Enable TTS module for iOS (was desktop-only) - Add iOS-specific path handling for model storage - Build ONNX Runtime 1.22.2 from source with all dependencies statically linked - Add retry logic and combined static library creation - Update GitHub Actions to build and cache ONNX Runtime for iOS Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Add build-ios-onnxruntime-all.sh to build ONNX Runtime for device + simulator - Add setup-ios-cargo-config.sh to generate .cargo/config.toml with correct paths - Add justfile commands: ios-build-onnxruntime, ios-setup-cargo-config, ios-fix-arch - Fix arm64-sim Xcode architecture issue in project.pbxproj - Configure vite to bind to 0.0.0.0 so iOS simulator can connect - Add comprehensive docs/ios-tts-local-development.md guide Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
…own Mode - Add webkitAudioContext fallback for iOS Safari compatibility - Handle suspended audio context state (iOS requires user interaction) - Show user-friendly error when AudioContext is unavailable (Lockdown Mode) - Add playbackError state and clearPlaybackError to TTS context - Display dismissible error alert in UnifiedChat when TTS playback fails Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
AnthonyRonning
force-pushed
the
ios-tts-working
branch
from
2f06b7e to
f06e242
Compare
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Summary
This PR adds Text-to-Speech (TTS) support for iOS using ONNX Runtime, bringing feature parity with the desktop version.
Changes
1. iOS TTS Support with ONNX Runtime
2. CI/CD Updates
3. Local Development Tooling
build-ios-onnxruntime-all.shto build ONNX Runtime for device + simulatorsetup-ios-cargo-config.shto generate .cargo/config.toml with correct pathsios-build-onnxruntime,ios-setup-cargo-config,ios-fix-archdocs/ios-tts-local-development.mdguide4. iOS Compatibility Fixes
Testing
Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.