Add mobile play UI and iOS TestFlight scaffold#60
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds iOS native app support for LLMQuest by creating a Swift UIKit application that serves the Play web app locally over a custom URL scheme. The changes span Xcode project configuration, Swift app lifecycle and WebView setup, site asset staging infrastructure, Play web app media and sharing enhancements, UI restructuring, build system updates, and comprehensive testing. ChangesiOS Native Play App with Local Site Serving
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 unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a native iOS wrapper for the LLM Quest experience, utilizing WKWebView and a custom lqb:// scheme to serve local site assets. It includes new documentation for TestFlight builds, Xcode project configurations, and Python tests to ensure asset and build integrity. The web interface has been significantly updated with a responsive 'play shell' layout, including support for local images and audio playback. Review feedback recommends improving the custom scheme handler's error responses with proper HTTP 404 status codes, resolving a potential double-extension bug in media URL generation, and optimizing audio preloading to 'auto' for better mobile responsiveness.
# Conflicts: # site/play/app.js # site/play/app.jsx
- WKScriptMessageHandler routes downloads through UIActivityViewController - downloadJson/downloadCanvas detect iOS context via webkit.messageHandlers - Disable ENABLE_USER_SCRIPT_SANDBOXING for stage_site.sh access to ../site - Merge master trace export (buildHumanTrace, Export Trace JSON button)
![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: d64e82b62d
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ios/LLMQuest.xcodeproj/project.pbxproj (1)
215-215: ⚡ Quick winUser script sandboxing disabled for site staging access.
ENABLE_USER_SCRIPT_SANDBOXING = NOis set to allowstage_site.shto access source files outside the build sandbox. This is necessary for the staging script to copy site assets from../siteinto the app bundle.This is a standard configuration for build scripts that need source tree access, and the staging script is read-only.
Also applies to: 276-276
🤖 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 `@ios/LLMQuest.xcodeproj/project.pbxproj` at line 215, The project currently disables Xcode user script sandboxing (ENABLE_USER_SCRIPT_SANDBOXING = NO) to let stage_site.sh read ../site; instead re-enable sandboxing by setting ENABLE_USER_SCRIPT_SANDBOXING = YES and modify the Run Script build phase that calls stage_site.sh to declare the external resource(s) as explicit input file(s) (e.g., add ${PROJECT_DIR}/../site or specific files to the script’s Input Files) or change the script to copy required assets into the allowed sandboxed paths before packaging; update stage_site.sh references accordingly so the build phase and ENABLE_USER_SCRIPT_SANDBOXING stay consistent.ios/LLMQuest/QuestWebViewController.swift (1)
84-89: ⚡ Quick winConsider surfacing file write errors to the user.
The file write failure is silently ignored (lines 87-89). Users won't receive feedback if sharing fails due to disk space or permissions issues. Consider presenting an alert or status message on catch.
💡 Proposed enhancement
do { try fileData.write(to: fileURL, options: .atomic) presentShareSheet(items: [fileURL]) } catch { - // File write failed — fall through silently. + let alert = UIAlertController( + title: "Share Failed", + message: "Unable to save file for sharing: \(error.localizedDescription)", + preferredStyle: .alert + ) + alert.addAction(UIAlertAction(title: "OK", style: .default)) + present(alert, animated: true) }🤖 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 `@ios/LLMQuest/QuestWebViewController.swift` around lines 84 - 89, The catch block in the do/try when writing fileData to fileURL currently swallows errors; update the catch to surface the failure to the user by presenting an alert or status message instead of falling through silently. Locate the do { try fileData.write(to: fileURL, options: .atomic); presentShareSheet(items: [fileURL]) } catch { ... } block and replace the empty catch with code that uses a UIAlertController (or your app's existing error UI) to show a user-facing message that includes error.localizedDescription and a friendly title, and ensure the alert is presented from QuestWebViewController so users see why sharing failed.
🤖 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.
Nitpick comments:
In `@ios/LLMQuest.xcodeproj/project.pbxproj`:
- Line 215: The project currently disables Xcode user script sandboxing
(ENABLE_USER_SCRIPT_SANDBOXING = NO) to let stage_site.sh read ../site; instead
re-enable sandboxing by setting ENABLE_USER_SCRIPT_SANDBOXING = YES and modify
the Run Script build phase that calls stage_site.sh to declare the external
resource(s) as explicit input file(s) (e.g., add ${PROJECT_DIR}/../site or
specific files to the script’s Input Files) or change the script to copy
required assets into the allowed sandboxed paths before packaging; update
stage_site.sh references accordingly so the build phase and
ENABLE_USER_SCRIPT_SANDBOXING stay consistent.
In `@ios/LLMQuest/QuestWebViewController.swift`:
- Around line 84-89: The catch block in the do/try when writing fileData to
fileURL currently swallows errors; update the catch to surface the failure to
the user by presenting an alert or status message instead of falling through
silently. Locate the do { try fileData.write(to: fileURL, options: .atomic);
presentShareSheet(items: [fileURL]) } catch { ... } block and replace the empty
catch with code that uses a UIAlertController (or your app's existing error UI)
to show a user-facing message that includes error.localizedDescription and a
friendly title, and ensure the alert is presented from QuestWebViewController so
users see why sharing failed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9463cebf-5491-4fa8-ba7e-7af18ba7b282
⛔ Files ignored due to path filters (30)
ios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-1024.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-120.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-152.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-167.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-180.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-20.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-29.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-40.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-58.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-60.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-76.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-80.pngis excluded by!**/*.pngios/LLMQuest/Assets.xcassets/AppIcon.appiconset/icon-87.pngis excluded by!**/*.pngsite/play/questplay/background.jpgis excluded by!**/*.jpgsite/play/questplay/frame-bottom.pngis excluded by!**/*.pngsite/play/questplay/frame-left-bottom-2.pngis excluded by!**/*.pngsite/play/questplay/frame-left-bottom.pngis excluded by!**/*.pngsite/play/questplay/frame-left-top-2.pngis excluded by!**/*.pngsite/play/questplay/frame-left-top.pngis excluded by!**/*.pngsite/play/questplay/frame-left.pngis excluded by!**/*.pngsite/play/questplay/frame-right-bottom-2.pngis excluded by!**/*.pngsite/play/questplay/frame-right-bottom.pngis excluded by!**/*.pngsite/play/questplay/frame-right-top-2.pngis excluded by!**/*.pngsite/play/questplay/frame-right-top.pngis excluded by!**/*.pngsite/play/questplay/frame-right.pngis excluded by!**/*.pngsite/play/questplay/frame-top.pngis excluded by!**/*.pngsite/play/questplay/frame.pngis excluded by!**/*.pngsite/play/vendor/pako-2.1.0.min.jsis excluded by!**/*.min.jssite/play/vendor/react-18.3.1.production.min.jsis excluded by!**/*.min.jssite/play/vendor/react-dom-18.3.1.production.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (25)
.github/workflows/ci.ymldocs/IOS_TESTFLIGHT.mdios/LLMQuest.xcodeproj/project.pbxprojios/LLMQuest.xcodeproj/xcshareddata/xcschemes/LLMQuest.xcschemeios/LLMQuest/AppDelegate.swiftios/LLMQuest/Assets.xcassets/AppIcon.appiconset/Contents.jsonios/LLMQuest/Assets.xcassets/Contents.jsonios/LLMQuest/Info.plistios/LLMQuest/LocalSiteSchemeHandler.swiftios/LLMQuest/PrivacyInfo.xcprivacyios/LLMQuest/QuestWebViewController.swiftios/LLMQuest/SceneDelegate.swiftios/LLMQuest/StageSiteInputs.xcfilelistios/export/ExportOptions.plist.templateios/scripts/stage_site.shllm_quest_benchmark/tests/core/test_analyzer.pyllm_quest_benchmark/tests/test_ios_testflight.pyllm_quest_benchmark/tests/test_play_share_social.pypackage.jsonscripts/import_human_trace.pysite/play.htmlsite/play/app.jssite/play/app.jsxsite/play/vendor/NOTICE.mdsite/play/vendor/bootstrap-5.3.3.min.css
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
uv run pytest llm_quest_benchmark/tests/test_ios_testflight.py llm_quest_benchmark/tests/test_play_share_social.py llm_quest_benchmark/tests/test_play_quest_index.py -xpnpm run buildgit diff --checkNote: local TestFlight archive/upload is blocked here because this Mac has Command Line Tools selected and no full
/Applications/Xcode.app.Summary by CodeRabbit
Release Notes
New Features
Documentation
Style