-
Notifications
You must be signed in to change notification settings - Fork 99
[iOS-SDK] [kotlin-SDK] Runanywhere-core consumption in Android and iOS #206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ities - Added LlamaCPPRuntime module to support text generation using GGUF models with Metal acceleration. - Implemented LlamaCPPService and LlamaCPPCoreAdapter for seamless integration with the RunAnywhere SDK. - Updated Package.swift to replace LLM.swift with RunAnywhereLlamaCPP, ensuring compatibility and improved performance. - Removed references to LLM.swift to avoid symbol conflicts and streamline the SDK. - Enhanced project structure with new JNI bindings and Kotlin support for LlamaCPP on Android. This commit significantly enhances the SDK's text generation capabilities, providing a robust alternative for LLM inference.
|
Caution Review failedThe pull request is closed. 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. WalkthroughReplaces LLMSwift/llama.cpp native/JNI integration with a unified RunAnywhereCore distribution: removes the llama-jni submodule and legacy LLMSwift adapters/providers, adds a C bridge (CRunAnywhereCore) and RunAnywhereCore binary target, and introduces new Kotlin core JNI and LlamaCPP backend modules with download/packaging tooling and updated Swift package manifests and examples. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client/App
participant SDK as RunAnywhere (Swift/Kotlin)
participant CBridge as CRunAnywhereCore (C API)
participant Native as RunAnywhereCore (XCFramework / native libs)
App->>SDK: registerAdapter(LlamaCPPCoreAdapter) / request generation
SDK->>CBridge: ra_create_backend("llamacpp") / ra_initialize(config)
CBridge->>Native: initialize backend, load native libs
Native-->>CBridge: backend ready / handles
CBridge-->>SDK: backend handle / success
SDK->>CBridge: ra_text_load_model(handle, modelPath)
CBridge->>Native: load model
Native-->>CBridge: model loaded
SDK->>CBridge: ra_text_generate_stream(..., callback)
CBridge->>Native: start streaming generate
Native-->>CBridge: token chunk
CBridge-->>SDK: token callback
SDK-->>App: streamed tokens
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Specific areas to focus review on:
Possibly related issues
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (15)
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 |
- Added CRunAnywhereCore target to expose unified C APIs for Swift integration. - Created necessary header files and module map for C interoperability. - Updated Package.swift to include CRunAnywhereCore as a dependency for ONNXRuntime and LlamaCPPRuntime. - Removed deprecated LlamaCPP and ONNX headers to streamline the SDK structure. This commit enhances the SDK's architecture by providing a unified C interface, improving compatibility and performance across different backends.
…unAnywhereAIApp - Introduced new model registrations for Qwen 2.5 (0.5B Instruct Q6_K) and LiquidAI LFM2 (350M) with different memory requirements. - Updated the model registration list to enhance text generation capabilities within the application. This commit expands the model options available for text generation, improving the versatility of the RunAnywhereAI application.
- Changed default temperature from 0.7 to 0.8 in LlamaCPPGenerationConfig and SDKConstants to align with LLM.swift defaults. - Updated documentation in GenerationOptions to reflect the new default temperature value. This commit ensures consistency in temperature settings across the SDK, improving user experience and predictability in text generation behavior.
- Added NativeTypes.kt and RunAnywhereBridge.kt to provide a JNI interface for the RunAnywhere Core C API, enhancing interoperability with Kotlin. - Updated LlamaCppCoreService to align default temperature values with LLM.swift, changing from 0.7 to 0.8 for consistency. - Removed LLMSwift related files to streamline the SDK and eliminate redundancy. This commit enhances the SDK's architecture by integrating a native bridge and ensuring consistent configuration across components, improving overall functionality and user experience.
…ructure - Removed LLMSwift library and related targets from Package.swift and Package.resolved to eliminate redundancy and avoid symbol conflicts. - Updated README and documentation to reflect the transition to LlamaCPPRuntime for GGUF model support. - Adjusted import statements and service provider registrations in example applications to use LlamaCPPRuntime. - Enhanced project organization by removing deprecated files and ensuring all references align with the new architecture. This commit improves the SDK's clarity and usability by consolidating LLM capabilities under the LlamaCPPRuntime, enhancing overall performance and maintainability.
- Introduced the RunAnywhere Core JNI module, providing a unified JNI bridge for all backends, including ONNX and LlamaCPP. - Updated settings.gradle.kts to include the new JNI module and adjusted project directories accordingly. - Enhanced LlamaCppCoreService and ONNXCoreService to load the JNI bridge and respective backend libraries during initialization. - Refactored build.gradle.kts files for LlamaCPP and ONNX modules to depend on the new JNI module, ensuring streamlined access to shared JNI functionalities. This commit enhances the SDK's architecture by integrating a unified JNI layer, improving interoperability and maintainability across different backend implementations.
|
@ellipsis please review |
|
@greptile please review |
|
@coderabbitai please review |
|
@sanchitmonga22: I'll review the changes in this PR right away. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 263acb7 in 4 minutes and 4 seconds. Click for details.
- Reviewed
8179lines of code in68files - Skipped
0files when reviewing. - Skipped posting
13draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPCoreAdapter.swift:39
- Draft comment:
The memory usage check uses a fixed 70% threshold of physical memory. Consider documenting or making this threshold configurable for better flexibility in low‐memory devices. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment is suggesting either documentation or making the threshold configurable. However, the actual suggestion provided only adds a comment, which is documentation. The rules state "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." This comment is somewhat speculative - it says "consider" making it configurable, which is not a definitive issue. The 70% threshold is already somewhat self-documenting in the code. The suggestion to make it configurable is a quality improvement suggestion, but it's not clear that this is a definite problem that needs fixing. The comment doesn't point to a bug or clear issue, just a potential improvement. The comment could be valid if there's evidence that the hardcoded threshold causes real problems on low-memory devices. It's a reasonable code quality suggestion about configurability. The inline comment suggestion is minimal and could be helpful for future maintainers. While it's a reasonable suggestion, the comment uses "consider" which makes it speculative rather than definitive. There's no evidence presented that this threshold is problematic. The suggestion only adds a comment, not actual configurability, which doesn't fully address the stated concern. This falls into the category of a "nice to have" refactor rather than a clear code change requirement. This comment should be deleted. It's a speculative suggestion using "consider" and doesn't point to a definite issue. The actual suggestion only adds an inline comment, which is minimal value. There's no evidence that the hardcoded threshold is causing problems.
2. sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift:214
- Draft comment:
Good use of AsyncThrowingStream for streaming generation. Verify that the postponed release of the callback context reliably frees the unmanaged reference to prevent leaks. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPServiceProvider.swift:80
- Draft comment:
The provider registration logic is clearly organized and uses descriptive logging. The use of available models from RunAnywhere.availableModels is well integrated. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swift:88
- Draft comment:
The recursive file search and archive extraction functions are well implemented. Consider adding comments about maximum recursion depth to clarify support for sherpa-onnx model structures. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
5. sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXSTTService.swift:172
- Draft comment:
The fallback mechanism for batch transcription when streaming is not supported is a smart design choice. Ensure that performance is acceptable on long audio streams. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%None
6. sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXTTSService.swift:287
- Draft comment:
The conversion from float samples to 16-bit PCM with a WAV header is comprehensive. Confirm that header fields (e.g., block align, byte rate) meet WAV format specifications for compatibility. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
7. sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Constants/SDKConstants.swift:65
- Draft comment:
The SDK constants, including default temperature and memory thresholds, are clearly defined and consistent with LlamaCPP defaults. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
8. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Extensions/RunAnywhere+Frameworks.swift:140
- Draft comment:
Event publishing using asynchronous Tasks is a neat design. Consider reviewing the order of event publication to ensure downstream consumers receive events in the intended sequence. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
9. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:46
- Draft comment:
The GenerationOptions initializer sets default temperature to 0.8, matching LlamaCPP conventions. The API is clearly documented—well done. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
10. examples/ios/RunAnywhereAI/IOS_SAMPLE_APP_DOCUMENTATION.md:310
- Draft comment:
Typographical suggestion: The new bullet item- Models list (per framework):is missing a space between the hyphen and the text. For consistency with the other bullet points (e.g., lines 303 and 304), consider changing it to- Models list (per framework):(with an appropriate space after the hyphen, if intended). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. examples/ios/RunAnywhereAI/IOS_SAMPLE_APP_DOCUMENTATION.md:1817
- Draft comment:
Typo/lexicographical issue: The term 'LlamaCPPRuntime' may be missing a space or hyphen. If the intended name is 'LlamaCPP Runtime' or 'LlamaCPP-Runtime', please update accordingly for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This appears to be a stylistic preference comment rather than a substantive issue. The term "LlamaCPPRuntime" is used consistently throughout the entire documentation file (over 10 occurrences). The diff shows this is a deliberate rename from "LLMSwift" to "LlamaCPPRuntime". The comment suggests this might be a typo, but there's no evidence it's incorrect - it's clearly the intended name of the package/module. The naming convention of compound words without spaces or hyphens is common in programming (e.g., "WhisperKit", "RunAnywhere"). The comment is speculative ("may be missing") and asks the author to confirm/update, which violates the rule about not asking authors to confirm intentions. Could "LlamaCPPRuntime" actually be incorrect? Maybe there's an official naming convention I'm not aware of. Perhaps the actual package or module is named differently in the codebase, and this documentation is inconsistent with the actual implementation. Looking at the diff more carefully, line 1649 shows this is referencing a local package path: "Local package: ../../sdk/runanywhere-swift/Sources/LlamaCPPRuntime". This suggests "LlamaCPPRuntime" is the actual directory/module name in the codebase. The consistent usage throughout the document and the reference to an actual file path strongly indicates this is the correct, intentional name, not a typo. This comment should be deleted. "LlamaCPPRuntime" is used consistently throughout the documentation and references an actual module/package name. The comment is speculative, asks the author to confirm/update based on a stylistic preference, and there's no evidence of an actual error. This violates the rules about speculative comments and asking authors to confirm intentions.
12. sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPServiceProvider.swift:63
- Draft comment:
Typo: The string "smollm" (line 63) might be a misspelling. Did you mean "smalllm" or a similar correct identifier? - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% "SmolLM" is a legitimate model name - it's a family of small language models released by Hugging Face. The spelling is intentional (using "smol" as internet slang for "small"). This is not a typo. The automated tool has incorrectly flagged this as a spelling error. The comment is incorrect and should be deleted. This is a clear case where the tool doesn't have the domain knowledge about actual model names in the LLM ecosystem. Could there be a scenario where the author actually did mean "smalllm" and made a mistake? Perhaps there's a different model family I'm not aware of that uses "smalllm" instead of "smollm"? While it's theoretically possible, "SmolLM" is a well-known model family from Hugging Face, and the context of this code (checking for common GGUF model names) strongly suggests the author knows what they're doing. The use of "smollm" alongside other legitimate model names like "mistral", "llama", "phi", "qwen", and "gemma" indicates this is intentional. There's no evidence this is a typo. This comment should be deleted. "SmolLM" is a legitimate model name from Hugging Face, not a typo. The automated tool incorrectly flagged this as a spelling error due to lack of domain knowledge about LLM model names.
13. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:40
- Draft comment:
Typo: It looks like there's an extra slash in the documentation comment. Consider removing the extra '/' in front of '- enableRealTimeTracking: Enable real-time cost tracking (default: true)'. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_SRoXPgBwI51yaeer
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
examples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/RunAnywhereApplication.kt (2)
33-35: ReplaceGlobalScope.launchwith an Application-scopedCoroutineScope.
GlobalScopeis an anti-pattern in Android that can cause memory leaks and makes it impossible to properly cancel work when the application is destroyed. Instead, create and manage a customCoroutineScopetied to the Application lifecycle.Apply this diff to create a proper coroutine scope:
+import kotlinx.coroutines.CoroutineScope + class RunAnywhereApplication : Application() { + private val applicationScope = CoroutineScope(Dispatchers.Main.immediate) + @Volatile private var isSDKInitialized = false @Volatile private var initializationError: Throwable? = null override fun onCreate() { super.onCreate() Log.i("RunAnywhereApp", "🏁 App launched, initializing SDK...") // Initialize SDK asynchronously to match iOS pattern - kotlinx.coroutines.GlobalScope.launch(Dispatchers.IO) { + applicationScope.launch(Dispatchers.IO) { initializeSDK() } } + + override fun onTrimMemory(level: Int) { + super.onTrimMemory(level) + // Allow cancellation of pending work during memory pressure + }
187-193: Fix misleading comment for QWEN model registration.Line 187 has a copy-paste error: the comment says "Llama 3.2 1B - good quality" but the code registers a QWEN 2 model.
- // Llama 3.2 1B - good quality + // Qwen 2.0 0.5B - lightweight model addModelFromURL( url = "https://huggingface.co/Qwen/Qwen2-0.5B-Instruct-GGUF/resolve/main/qwen2-0_5b-instruct-q4_0.gguf", name = "QWEN 2 0.5B Q4_0 Instruct", type = "LLM" ) - Log.i("RunAnywhereApp", "✅ Registered: QWEN 2 0.5B Instruct Q6_K Q4_0") + Log.i("RunAnywhereApp", "✅ Registered: QWEN 2 0.5B Q4_0 Instruct")sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXTTSService.swift (1)
7-7: Based on my verification, the review comment is correct and identifies a genuine critical issue. Here are the key findings:Verified Issues:
- Unnecessary NSObject inheritance — The
TTSServiceprotocol only requiresAnyObject, notNSObject. NSObject is unnecessary baggage.- Actual data races — Multiple async functions (
initialize(),synthesize(),stop(),cleanup()) modify shared mutable state (_isSynthesizing,_isReady,backendHandle,modelPath) without isolation.- @unchecked Sendable danger — The annotation bypasses Swift's concurrency safety checks, masking potential races when concurrent tasks call these methods.
- Actor conversion is viable — Removing NSObject and converting to an
actoris completely feasible and solves all race conditions automatically.The review comment's recommendation is aligned with the coding guidelines (use Swift 6 concurrency primitives, not unsafe annotations like
@unchecked Sendable).
Critical: Unsafe use of
@unchecked Sendablewith shared mutable state accessed from async contexts.The class marks itself
@unchecked Sendablewhile maintaining non-isolated mutable state (_isSynthesizing,_isReady,backendHandle,modelPath) that is modified from async functions (initialize(),synthesize(),stop(),cleanup()). This creates potential data races when concurrent calls occur.Per coding guidelines, use Swift 6 concurrency primitives instead of unsafe annotations. The recommended approach:
Convert to an Actor — This provides automatic isolation and data-race protection:
-public final class ONNXTTSService: NSObject, TTSService, @unchecked Sendable { +public actor ONNXTTSService: TTSService { private let logger = SDKLogger(category: "ONNXTTSService") private var backendHandle: ra_backend_handle? private var modelPath: String? private var _isSynthesizing: Bool = false private var _isReady: Bool = falseNote:
NSObjectinheritance is not required by theTTSServiceprotocol (which only requiresAnyObject), so it can be safely removed. Theactorkeyword satisfies the class requirement while providing automatic thread-safety for all properties.Also applies to: 10-13, 159-159, 168-168, 230-231, 271-272
sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXSTTService.swift (1)
7-7: Mutable state in ONNXSTTService accesses from async contexts without proper Swift 6 concurrency isolation—data race risk confirmed.Verification confirms the review comment is accurate. Both ONNXSTTService and WhisperKitService (the two STTService implementations) modify mutable properties (
backendHandle,streamHandle,_isReady,_currentModel,_supportsStreamingin ONNXSTTService;isInitialized,whisperKit,currentModelPathin WhisperKitService) from async protocol methods without synchronization.Verified issues:
streamTranscribe()modifiesstreamHandleat lines 222 and 225 while processing concurrent chunkscleanup()can race withinitialize()andstreamTranscribe()modifying shared stateSTTServiceprotocol lacks isolation attributes, allowing concurrent method calls- Direct instantiation (shown in ONNXAdapter and documentation examples) bypasses STTComponent's @mainactor wrapper
While STTComponent applies
@MainActorisolation when services are accessed through the framework's standard path, direct instantiation patterns exist in the codebase that bypass this protection. The suggested fixes (@mainactor or actor conversion) are compatible with the protocol, though the protocol itself should also be annotated to enforce isolation across all implementations.The issue affects multiple STTService implementations consistently.
sdk/runanywhere-swift/Package.resolved (1)
30-38: Critical finding: FluidAudio branch dependency is a regression that contradicts the yanked 0.15.1 fix.The CHANGELOG reveals a significant issue:
- Version 0.15.1 attempted to fix this exact problem, changing FluidAudio from
branch: "main"tofrom: "0.5.0"for SPM compatibility- Version 0.15.1 was yanked due to SPM resolution issues with FluidAudio
- Current code has reverted back to
branch: "main"in both Package.swift (line 84) and Package.resolvedThe code now reintroduces the branch dependency that caused 0.15.1 to fail. Using
branch: "main"creates unpredictable behavior for SDK consumers and defeats the purpose of version pinning. This must either be resolved to use a tagged version (e.g.,from: "0.5.0") or documented as an intentional trade-off with clear reasoning.
🧹 Nitpick comments (13)
sdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.kt (1)
90-96: Silent error swallowing may mask genuine library loading failures.The
tryLoadLibraryfunction catches allUnsatisfiedLinkErrorexceptions silently. While this is reasonable for libraries that might be statically linked, it could mask genuine failures (e.g., missing library, ABI incompatibility, missing dependencies).Consider logging the exception at debug level or differentiating between "already loaded" and actual failures:
private fun tryLoadLibrary(name: String) { try { System.loadLibrary(name) } catch (e: UnsatisfiedLinkError) { - // May already be loaded or linked statically - this is fine + // May already be loaded or linked statically + // Log at debug level for troubleshooting + android.util.Log.d("RunAnywhereBridge", "Library $name load skipped: ${e.message}") } }Note: For KMP compatibility, consider using a platform-agnostic logging approach or
expect/actualfor the logging call.sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts (1)
76-82: Fragile parent navigation for dependency resolution.Using
project.parent!!.parent!!is brittle and will break if the module hierarchy changes. Consider using an explicit project path instead.val commonMain by getting { dependencies { // Core SDK dependency for interfaces and models - api(project.parent!!.parent!!) + api(project(":sdk:runanywhere-kotlin")) implementation(libs.kotlinx.coroutines.core) implementation(libs.kotlinx.serialization.json) } }sdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.kts (1)
74-80: Same fragile parent navigation pattern as llamacpp module.This uses the same brittle
project.parent!!.parent!!pattern. Apply the same fix for consistency.val commonMain by getting { dependencies { // Core SDK dependency for result types (NativeTTSSynthesisResult, etc.) - api(project.parent!!.parent!!) + api(project(":sdk:runanywhere-kotlin")) implementation(libs.kotlinx.coroutines.core) } }examples/android/RunAnywhereAI/app/build.gradle.kts (1)
128-137: Consider aligning pickFirsts with active ABIs.The
pickFirstsconfiguration declares duplicate resolution for four ABIs (arm64-v8a, armeabi-v7a, x86, x86_64), but line 35 restrictsabiFiltersto onlyarm64-v8a. This creates unnecessary configuration for ABIs that won't be included in the build.If only arm64-v8a is needed currently, simplify:
pickFirsts += listOf( "lib/arm64-v8a/librunanywhere_bridge.so", - "lib/arm64-v8a/librunanywhere_jni.so", - "lib/armeabi-v7a/librunanywhere_bridge.so", - "lib/armeabi-v7a/librunanywhere_jni.so", - "lib/x86/librunanywhere_bridge.so", - "lib/x86/librunanywhere_jni.so", - "lib/x86_64/librunanywhere_bridge.so", - "lib/x86_64/librunanywhere_jni.so" + "lib/arm64-v8a/librunanywhere_jni.so" )Alternatively, if you're preparing for multi-ABI support, add a comment explaining the forward-looking configuration.
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppServiceProvider.kt (1)
94-109: Memory check threshold may be too aggressive for mobile devices.Line 98 checks if
memoryRequired > availableMemory * 0.8, which only warns but doesn't block. However,getAvailableSystemMemory()at line 146 usesRuntime.getRuntime().maxMemory()which returns heap memory, not physical memory. This could be misleading for GGUF models that typically use native memory outside the JVM heap.Consider clarifying the memory check or adding a comment explaining the limitation:
private fun getAvailableSystemMemory(): Long { + // Note: Returns JVM heap max, not total physical memory + // GGUF models use native memory - this is a rough approximation return Runtime.getRuntime().maxMemory() }sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift (1)
38-49: Documentation reference may be outdated.Line 38 references "matches LLM.swift" but the PR removes LLMSwift in favor of LlamaCPP. Consider updating the comment to reference the new backend or remove the file reference:
- /// - temperature: Sampling temperature (default: 0.8, matches LLM.swift) + /// - temperature: Sampling temperature (default: 0.8)sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPCoreAdapter.swift (2)
8-8: Consider making the classfinal.
LlamaCPPCoreAdapterdoesn't appear to be designed for subclassing. Marking itfinalprevents unintended inheritance and can enable compiler optimizations.-public class LlamaCPPCoreAdapter: UnifiedFrameworkAdapter { +public final class LlamaCPPCoreAdapter: UnifiedFrameworkAdapter {
96-105: Quantization matching is case-sensitive.The
isQuantizationSupportedcheck uses exact string matching. Ifquantization.rawValuecomes in a different case (e.g.,"q4_k_m"lowercase), the check will fail. Consider case-insensitive comparison.private func isQuantizationSupported(_ quantization: String) -> Bool { let supportedQuantizations = [ "Q2_K", "Q3_K_S", "Q3_K_M", "Q3_K_L", "Q4_0", "Q4_1", "Q4_K_S", "Q4_K_M", "Q5_0", "Q5_1", "Q5_K_S", "Q5_K_M", "Q6_K", "Q8_0", "IQ2_XXS", "IQ2_XS", "IQ3_S", "IQ3_XXS", "IQ4_NL", "IQ4_XS" ] - return supportedQuantizations.contains(quantization) + return supportedQuantizations.contains { $0.caseInsensitiveCompare(quantization) == .orderedSame } }sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift (1)
64-64: Consider making the classfinal.
LlamaCPPServicedoesn't appear designed for subclassing. Marking itfinalimproves clarity and enables compiler optimizations.-public class LlamaCPPService { +public final class LlamaCPPService {sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt (2)
48-52: Consider lazy initialization for library loading.Per coding guidelines, lazy initialization is preferred to avoid memory pressure. Currently, the library is loaded eagerly in the init block. Consider deferring to
initialize()unless immediate availability checks are required.Based on learnings, lazy initialization is preferred for service dependencies.
init { - // Load JNI bridge and LlamaCPP backend libraries on construction - RunAnywhereBridge.loadLibrary() - RunAnywhereBridge.loadBackend("llamacpp") + // Libraries loaded lazily in initialize() } + +suspend fun initialize(configJson: String? = null) { + withContext(Dispatchers.IO) { + mutex.withLock { + if (backendHandle != 0L) { + return@withContext + } + + // Load JNI bridge and LlamaCPP backend libraries + RunAnywhereBridge.loadLibrary() + RunAnywhereBridge.loadBackend("llamacpp") + + // Create backend + backendHandle = RunAnywhereBridge.nativeCreateBackend("llamacpp") + // ... rest of initialization
101-139: Property accessors are not mutex-protected.The read accessors (
isInitialized,supportedCapabilities,deviceType,memoryUsage) accessbackendHandlewithout the mutex. While these are read operations, they could observe torn reads during concurrent mutations. Consider documenting thread-safety expectations or using consistent locking.sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt (2)
28-28: Consider lazy initialization forcoreService.Per coding guidelines, lazy initialization is preferred for service dependencies. The
coreServiceis eagerly instantiated on construction but could be created lazily ininitialize().Based on learnings, lazy initialization avoids memory pressure.
-private val coreService = LlamaCppCoreService() +private var coreService: LlamaCppCoreService? = null actual override suspend fun initialize(modelPath: String?) = withContext(Dispatchers.IO) { // ... + if (coreService == null) { + coreService = LlamaCppCoreService() + } // Initialize the core service - coreService.initialize() + coreService!!.initialize()
248-251: Token estimation is approximate.The
estimateTokenCountuses a ~4 chars/token heuristic. This is acceptable for rough context-fitting checks but may be inaccurate for non-English text or code. Consider documenting this limitation or using the model's actual tokenizer when available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (68)
.gitmodules(0 hunks)Package.resolved(0 hunks)Package.swift(0 hunks)README.md(5 hunks)examples/android/RunAnywhereAI/app/build.gradle.kts(2 hunks)examples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/RunAnywhereApplication.kt(2 hunks)examples/android/RunAnywhereAI/settings.gradle.kts(1 hunks)examples/ios/RunAnywhereAI/IOS_SAMPLE_APP_DOCUMENTATION.md(12 hunks)examples/ios/RunAnywhereAI/Package.resolved(1 hunks)examples/ios/RunAnywhereAI/Package.swift(2 hunks)examples/ios/RunAnywhereAI/RunAnywhereAI.xcodeproj/project.pbxproj(4 hunks)examples/ios/RunAnywhereAI/RunAnywhereAI.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved(1 hunks)examples/ios/RunAnywhereAI/RunAnywhereAI/App/RunAnywhereAIApp.swift(5 hunks)examples/ios/RunAnywhereAI/docs/SDK_CONSUMPTION_FLOWS.md(3 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.kts(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.kt(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppServiceProvider.kt(5 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.kts(2 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/onnx/ONNXCoreService.kt(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/build.gradle.kts(0 hunks)sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppModule.kt(0 hunks)sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppProvider.kt(0 hunks)sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LLamaAndroid.kt(0 hunks)sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppModuleActual.kt(0 hunks)sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt(0 hunks)sdk/runanywhere-kotlin/native/llama-jni/CMakeLists.txt(0 hunks)sdk/runanywhere-kotlin/native/llama-jni/build-native.sh(0 hunks)sdk/runanywhere-kotlin/native/llama-jni/llama-cpp-setup.md(0 hunks)sdk/runanywhere-kotlin/native/llama-jni/llama.cpp(0 hunks)sdk/runanywhere-kotlin/native/llama-jni/src/cpu_features.cpp(0 hunks)sdk/runanywhere-kotlin/native/llama-jni/src/cpu_features.h(0 hunks)sdk/runanywhere-kotlin/native/llama-jni/src/llama-android.cpp(0 hunks)sdk/runanywhere-kotlin/settings.gradle.kts(1 hunks)sdk/runanywhere-swift/.gitignore(1 hunks)sdk/runanywhere-swift/ARCHITECTURE.md(4 hunks)sdk/runanywhere-swift/CHANGELOG.md(2 hunks)sdk/runanywhere-swift/Package.resolved(6 hunks)sdk/runanywhere-swift/Package.swift(5 hunks)sdk/runanywhere-swift/README.md(6 hunks)sdk/runanywhere-swift/Sources/CRunAnywhereCore/dummy.c(1 hunks)sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/module.modulemap(1 hunks)sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_core.h(1 hunks)sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_llamacpp_bridge.h(1 hunks)sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_onnx_bridge.h(14 hunks)sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_types.h(1 hunks)sdk/runanywhere-swift/Sources/CRunAnywhereONNX/dummy.c(0 hunks)sdk/runanywhere-swift/Sources/CRunAnywhereONNX/include/module.modulemap(0 hunks)sdk/runanywhere-swift/Sources/CRunAnywhereONNX/include/types.h(0 hunks)sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftAdapter.swift(0 hunks)sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftError.swift(0 hunks)sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftService.swift(0 hunks)sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftServiceProvider.swift(0 hunks)sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftTemplateResolver.swift(0 hunks)sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPCoreAdapter.swift(1 hunks)sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPRuntime.swift(1 hunks)sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift(1 hunks)sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPServiceProvider.swift(1 hunks)sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swift(2 hunks)sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXSTTService.swift(1 hunks)sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXTTSService.swift(1 hunks)sdk/runanywhere-swift/Sources/RunAnywhere/Components/LLM/LLMComponent.swift(2 hunks)sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Constants/SDKConstants.swift(1 hunks)sdk/runanywhere-swift/Sources/RunAnywhere/Public/Extensions/RunAnywhere+Frameworks.swift(1 hunks)sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift(2 hunks)
💤 Files with no reviewable changes (24)
- sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftError.swift
- sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppModuleActual.kt
- sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftTemplateResolver.swift
- Package.resolved
- sdk/runanywhere-swift/Sources/CRunAnywhereONNX/include/types.h
- .gitmodules
- sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppModule.kt
- sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftService.swift
- sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftServiceProvider.swift
- sdk/runanywhere-kotlin/native/llama-jni/src/cpu_features.cpp
- sdk/runanywhere-kotlin/native/llama-jni/CMakeLists.txt
- sdk/runanywhere-swift/Sources/LLMSwift/LLMSwiftAdapter.swift
- sdk/runanywhere-kotlin/native/llama-jni/build-native.sh
- Package.swift
- sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/build.gradle.kts
- sdk/runanywhere-kotlin/native/llama-jni/llama.cpp
- sdk/runanywhere-kotlin/native/llama-jni/src/llama-android.cpp
- sdk/runanywhere-kotlin/native/llama-jni/llama-cpp-setup.md
- sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt
- sdk/runanywhere-kotlin/native/llama-jni/src/cpu_features.h
- sdk/runanywhere-swift/Sources/CRunAnywhereONNX/include/module.modulemap
- sdk/runanywhere-swift/Sources/CRunAnywhereONNX/dummy.c
- sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppProvider.kt
- sdk/runanywhere-kotlin/modules/runanywhere-llm-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LLamaAndroid.kt
🧰 Additional context used
📓 Path-based instructions (4)
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Use the latest Swift 6 APIs always.
Do not use NSLock as it is outdated. Use Swift 6 concurrency primitives instead.
Files:
sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Constants/SDKConstants.swiftsdk/runanywhere-swift/Sources/ONNXRuntime/ONNXSTTService.swiftsdk/runanywhere-swift/Sources/ONNXRuntime/ONNXTTSService.swiftsdk/runanywhere-swift/Sources/RunAnywhere/Components/LLM/LLMComponent.swiftsdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swiftsdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swiftsdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPRuntime.swiftexamples/ios/RunAnywhereAI/Package.swiftsdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPServiceProvider.swiftsdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swiftsdk/runanywhere-swift/Sources/RunAnywhere/Public/Extensions/RunAnywhere+Frameworks.swiftexamples/ios/RunAnywhereAI/RunAnywhereAI/App/RunAnywhereAIApp.swiftsdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPCoreAdapter.swiftsdk/runanywhere-swift/Package.swift
sdk/runanywhere-kotlin/**/*.gradle.kts
📄 CodeRabbit inference engine (CLAUDE.md)
sdk/runanywhere-kotlin/**/*.gradle.kts: Kotlin version must be 2.1.21 or later for the Kotlin Multiplatform SDK.
JVM target must be 17 or later for Kotlin Multiplatform SDK.
Android minSdk must be 24 or later for Kotlin Multiplatform SDK.
Files:
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.kts
sdk/runanywhere-kotlin/**/src/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
sdk/runanywhere-kotlin/**/src/**/*.kt: All business logic, protocols, interfaces, and structures MUST be defined incommonMain/for Kotlin Multiplatform projects. Platform-specific implementations must use clear prefixes likeAndroidTTSService.kt,JvmTTSService.kt, etc.
NEVER make assumptions when implementing KMP code. ALWAYS refer to the iOS implementation as the definitive source of truth. Check the iOS implementation, copy the logic exactly, and adapt only for Kotlin syntax.
Useexpect/actualONLY for platform-specific implementations, not for business logic. Put all business logic incommonMain/.
Use Flow instead of AsyncSequence for reactive streams in Kotlin Multiplatform code.
Use coroutines and suspend functions instead of async/await patterns for asynchronous operations.
Use sealed classes for type-safe error handling in Kotlin code.
Always use data classes and structured types (enums, sealed classes) instead of strings for models and configuration.
Use theModuleRegistryprovider pattern for extensibility and plugin architecture instead of hard-coded dependencies.
Use EventBus for component communication instead of direct coupling between components.
Use Service Container pattern for centralized dependency injection with lazy initialization.
Use lazy initialization for service dependencies to avoid memory pressure and allow garbage collection.
Files:
sdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktsdk/runanywhere-kotlin/modules/runanywhere-core-onnx/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/onnx/ONNXCoreService.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppServiceProvider.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt
sdk/runanywhere-swift/**/Package.swift
📄 CodeRabbit inference engine (CLAUDE.md)
sdk/runanywhere-swift/**/Package.swift: iOS minimum deployment target must be 13.0 or later for Swift SDK.
Swift minimum version must be 5.9 or later for iOS SDK.
Files:
sdk/runanywhere-swift/Package.swift
🧠 Learnings (19)
📓 Common learnings
Learnt from: shubhammalhotra28
Repo: RunanywhereAI/runanywhere-sdks PR: 119
File: README.md:176-176
Timestamp: 2025-10-17T22:19:36.568Z
Learning: The RunAnywhere SDK package repository is located at `https://github.com/RunanywhereAI/runanywhere-sdks`. The repository was moved from `sdks` to `runanywhere-sdks`, so the correct URL to reference in documentation and installation instructions is `https://github.com/RunanywhereAI/runanywhere-sdks`.
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use the `ModuleRegistry` provider pattern for extensibility and plugin architecture instead of hard-coded dependencies.
Applied to files:
examples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/RunAnywhereApplication.ktsdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktsexamples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktsdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPServiceProvider.swiftexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppServiceProvider.kt
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use lazy initialization for service dependencies to avoid memory pressure and allow garbage collection.
Applied to files:
examples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/RunAnywhereApplication.ktexamples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/onnx/ONNXCoreService.ktsdk/runanywhere-kotlin/settings.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-swift/**/Package.swift : iOS minimum deployment target must be 13.0 or later for Swift SDK.
Applied to files:
sdk/runanywhere-swift/CHANGELOG.mdsdk/runanywhere-swift/Sources/CRunAnywhereCore/dummy.cexamples/ios/RunAnywhereAI/RunAnywhereAI.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedexamples/ios/RunAnywhereAI/Package.swiftsdk/runanywhere-swift/README.mdexamples/ios/RunAnywhereAI/RunAnywhereAI.xcodeproj/project.pbxprojexamples/ios/RunAnywhereAI/Package.resolvedsdk/runanywhere-swift/Package.resolvedREADME.mdsdk/runanywhere-swift/Package.swift
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-swift/**/Package.swift : Swift minimum version must be 5.9 or later for iOS SDK.
Applied to files:
sdk/runanywhere-swift/CHANGELOG.mdsdk/runanywhere-swift/Sources/CRunAnywhereCore/dummy.cexamples/ios/RunAnywhereAI/RunAnywhereAI.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedexamples/ios/RunAnywhereAI/IOS_SAMPLE_APP_DOCUMENTATION.mdexamples/ios/RunAnywhereAI/Package.swiftsdk/runanywhere-swift/README.mdexamples/ios/RunAnywhereAI/RunAnywhereAI.xcodeproj/project.pbxprojexamples/ios/RunAnywhereAI/Package.resolvedsdk/runanywhere-swift/Package.resolvedREADME.mdsdk/runanywhere-swift/Package.swift
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/androidMain/**/*.kt : Use platform-specific optimizations in their respective source directories: `androidMain/` for Android optimizations, `jvmMain/` for JVM optimizations.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktsexamples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-kotlin/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/jvmMain/**/*.kt : Use platform-specific optimizations in their respective source directories: `androidMain/` for Android optimizations, `jvmMain/` for JVM optimizations.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktsexamples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-kotlin/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/*.gradle.kts : Android minSdk must be 24 or later for Kotlin Multiplatform SDK.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktsexamples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktssdk/runanywhere-swift/Package.swift
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/*.gradle.kts : JVM target must be 17 or later for Kotlin Multiplatform SDK.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktsexamples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-kotlin/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/*.gradle.kts : Kotlin version must be 2.1.21 or later for the Kotlin Multiplatform SDK.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktsexamples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-kotlin/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : All business logic, protocols, interfaces, and structures MUST be defined in `commonMain/` for Kotlin Multiplatform projects. Platform-specific implementations must use clear prefixes like `AndroidTTSService.kt`, `JvmTTSService.kt`, etc.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktsexamples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppServiceProvider.kt
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use `expect/actual` ONLY for platform-specific implementations, not for business logic. Put all business logic in `commonMain/`.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-kotlin/settings.gradle.kts
📚 Learning: 2025-10-17T22:19:36.568Z
Learnt from: shubhammalhotra28
Repo: RunanywhereAI/runanywhere-sdks PR: 119
File: README.md:176-176
Timestamp: 2025-10-17T22:19:36.568Z
Learning: The RunAnywhere SDK package repository is located at `https://github.com/RunanywhereAI/runanywhere-sdks`. The repository was moved from `sdks` to `runanywhere-sdks`, so the correct URL to reference in documentation and installation instructions is `https://github.com/RunanywhereAI/runanywhere-sdks`.
Applied to files:
examples/android/RunAnywhereAI/app/build.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-swift/ARCHITECTURE.md
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use coroutines and suspend functions instead of async/await patterns for asynchronous operations.
Applied to files:
examples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-kotlin/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use Service Container pattern for centralized dependency injection with lazy initialization.
Applied to files:
examples/android/RunAnywhereAI/app/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/onnx/ONNXCoreService.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Use `./scripts/sdk.sh` for all Kotlin SDK operations instead of calling Gradle directly. It handles configuration and build complexity.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to **/*.swift : Use the latest Swift 6 APIs always.
Applied to files:
examples/ios/RunAnywhereAI/IOS_SAMPLE_APP_DOCUMENTATION.mdsdk/runanywhere-swift/Package.resolved
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to **/*.swift : Do not use NSLock as it is outdated. Use Swift 6 concurrency primitives instead.
Applied to files:
examples/ios/RunAnywhereAI/IOS_SAMPLE_APP_DOCUMENTATION.md
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/components/**/*.kt : Implement proper component lifecycle management with initialization, state tracking, and cleanup in BaseComponent.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt
🧬 Code graph analysis (5)
sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPServiceProvider.swift (2)
sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPCoreAdapter.swift (1)
canHandle(30-42)sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift (1)
initialize(102-128)
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt (3)
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/onnx/ONNXCoreService.kt (1)
ensureInitialized(345-352)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt (1)
generate(61-76)sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift (3)
generate(168-204)generateStream(211-278)cancel(281-285)
examples/ios/RunAnywhereAI/RunAnywhereAI/App/RunAnywhereAIApp.swift (2)
sdk/runanywhere-swift/Sources/RunAnywhere/Public/Extensions/RunAnywhere+Frameworks.swift (1)
registerFramework(31-67)sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Logging/Logger/SDKLogger.swift (1)
info(26-28)
sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPCoreAdapter.swift (2)
sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPServiceProvider.swift (1)
canHandle(37-80)sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift (5)
loadModel(133-148)initialize(102-128)generate(168-204)generateStream(211-278)unloadModel(151-159)
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt (2)
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt (2)
cleanup(40-40)initialize(33-33)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt (1)
initialize(67-96)
🪛 Clang (14.0.6)
sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_types.h
[error] 10-10: 'stddef.h' file not found
(clang-diagnostic-error)
sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_onnx_bridge.h
[error] 20-20: 'stddef.h' file not found
(clang-diagnostic-error)
sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_llamacpp_bridge.h
[error] 20-20: 'stddef.h' file not found
(clang-diagnostic-error)
| try { | ||
| logger.lifecycle("Downloading from: $downloadUrl") | ||
| URL(downloadUrl).openStream().use { input -> | ||
| zipFile.outputStream().use { output -> | ||
| input.copyTo(output) | ||
| } | ||
| } | ||
| logger.lifecycle("Downloaded: ${zipFile.length() / 1024}KB") | ||
| } catch (e: Exception) { | ||
| logger.error("Failed to download JNI libraries: ${e.message}") | ||
| logger.error("URL: $downloadUrl") | ||
| logger.lifecycle("") | ||
| logger.lifecycle("Options:") | ||
| logger.lifecycle(" 1. Check that version $nativeLibVersion exists in the releases") | ||
| logger.lifecycle(" 2. Build locally: cd runanywhere-core && ./scripts/build-android.sh all") | ||
| logger.lifecycle(" 3. Use local mode: ./gradlew build -Prunanywhere.native.local=true") | ||
| throw GradleException("Failed to download JNI libraries", e) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same timeout and integrity concerns as llamacpp module.
This download task has identical issues: no connection timeout on URL().openStream() and no checksum verification. Consider extracting this into a shared build logic plugin or convention to avoid duplication and ensure consistent security practices.
🤖 Prompt for AI Agents
In sdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.kts around
lines 212-229, the download logic uses URL.openStream() without connection/read
timeouts and performs no integrity check; change it to use a HttpURLConnection
(or equivalent) where you set reasonable connect and read timeouts and
explicitly close streams, compute and verify the downloaded file against a known
checksum (or download and verify a .sha256 signature) before proceeding, and
factor this logic into a shared Gradle build function/plugin used by both this
module and the llamacpp module to avoid duplication and ensure consistent
timeout and checksum behavior across projects.
| try { | ||
| logger.lifecycle("Downloading from: $downloadUrl") | ||
| URL(downloadUrl).openStream().use { input -> | ||
| zipFile.outputStream().use { output -> | ||
| input.copyTo(output) | ||
| } | ||
| } | ||
| logger.lifecycle("Downloaded: ${zipFile.length() / 1024}KB") | ||
| } catch (e: Exception) { | ||
| logger.error("Failed to download native libraries: ${e.message}") | ||
| logger.error("URL: $downloadUrl") | ||
| logger.lifecycle("") | ||
| logger.lifecycle("Options:") | ||
| logger.lifecycle(" 1. Check that version $nativeLibVersion exists in the releases") | ||
| logger.lifecycle(" 2. Build locally: cd runanywhere-core && ./scripts/build-android-backend.sh llamacpp") | ||
| logger.lifecycle(" 3. Use local mode: ./gradlew build -Prunanywhere.native.local=true") | ||
| throw GradleException("Failed to download native libraries", e) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add timeout and consider integrity verification for downloaded binaries.
The URL().openStream() call has no timeout, which could cause builds to hang indefinitely on network issues. Additionally, downloading native binaries without checksum verification poses a supply chain security risk.
Consider adding connection timeout and checksum verification:
+import java.net.HttpURLConnection
+
// Download the ZIP file
try {
logger.lifecycle("Downloading from: $downloadUrl")
- URL(downloadUrl).openStream().use { input ->
+ val connection = URL(downloadUrl).openConnection() as HttpURLConnection
+ connection.connectTimeout = 30_000 // 30 seconds
+ connection.readTimeout = 120_000 // 2 minutes for large files
+ connection.inputStream.use { input ->
zipFile.outputStream().use { output ->
input.copyTo(output)
}
}
logger.lifecycle("Downloaded: ${zipFile.length() / 1024}KB")
+ // TODO: Verify SHA256 checksum against expected value
} catch (e: Exception) {Committable suggestion skipped: line range outside the PR's diff.
...-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt
Outdated
Show resolved
Hide resolved
| fun generateFlow( | ||
| prompt: String, | ||
| systemPrompt: String? = null, | ||
| maxTokens: Int = 256, | ||
| temperature: Float = 0.8f // Match LLM.swift default | ||
| ): Flow<String> = callbackFlow { | ||
| try { | ||
| generateStream(prompt, systemPrompt, maxTokens, temperature) { token -> | ||
| trySend(token).isSuccess | ||
| } | ||
| } catch (e: Exception) { | ||
| close(e) | ||
| } | ||
| close() | ||
| awaitClose { cancel() } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect callbackFlow usage.
The close() call on line 296 terminates the flow, making the awaitClose block (line 297) run immediately. But close() should not be called before awaitClose. Instead, let awaitClose handle the flow termination naturally when the collector cancels.
fun generateFlow(
prompt: String,
systemPrompt: String? = null,
maxTokens: Int = 256,
temperature: Float = 0.8f
): Flow<String> = callbackFlow {
try {
generateStream(prompt, systemPrompt, maxTokens, temperature) { token ->
trySend(token).isSuccess
}
+ close() // Signal completion after streaming finishes
} catch (e: Exception) {
close(e)
}
- close()
awaitClose { cancel() }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun generateFlow( | |
| prompt: String, | |
| systemPrompt: String? = null, | |
| maxTokens: Int = 256, | |
| temperature: Float = 0.8f // Match LLM.swift default | |
| ): Flow<String> = callbackFlow { | |
| try { | |
| generateStream(prompt, systemPrompt, maxTokens, temperature) { token -> | |
| trySend(token).isSuccess | |
| } | |
| } catch (e: Exception) { | |
| close(e) | |
| } | |
| close() | |
| awaitClose { cancel() } | |
| } | |
| fun generateFlow( | |
| prompt: String, | |
| systemPrompt: String? = null, | |
| maxTokens: Int = 256, | |
| temperature: Float = 0.8f // Match LLM.swift default | |
| ): Flow<String> = callbackFlow { | |
| try { | |
| generateStream(prompt, systemPrompt, maxTokens, temperature) { token -> | |
| trySend(token).isSuccess | |
| } | |
| close() // Signal completion after streaming finishes | |
| } catch (e: Exception) { | |
| close(e) | |
| } | |
| awaitClose { cancel() } | |
| } |
🤖 Prompt for AI Agents
In
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt
around lines 283–298, remove the premature close() call (line ~296) that
terminates the callbackFlow before awaitClose runs; instead let awaitClose
handle cancellation/termination naturally, keeping the existing try/catch that
calls close(e) on exceptions and ensuring generateStream's completion or the
collector's cancellation is handled by awaitClose { cancel() } so the flow is
not closed early.
| // Include types from the canonical source | ||
| // Note: When built as xcframework, types.h is in same directory | ||
| // When building locally, it's in ../capabilities/ | ||
| #if __has_include("types.h") | ||
| #include "ra_types.h" | ||
| #else | ||
| #include "ra_types.h" | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conditional include is ineffective — both branches include the same file.
The __has_include("types.h") check tests for types.h, but both branches include ra_types.h. The comment indicates the intent was to handle different include paths for xcframework vs local builds, but the implementation doesn't achieve this.
If different paths are needed, consider:
-// Note: When built as xcframework, types.h is in same directory
-// When building locally, it's in ../capabilities/
-#if __has_include("types.h")
-#include "ra_types.h"
-#else
-#include "ra_types.h"
-#endif
+#include "ra_types.h"Or if path differentiation is actually required:
-#if __has_include("types.h")
-#include "ra_types.h"
+#if __has_include("ra_types.h")
+#include "ra_types.h"
+#elif __has_include("../capabilities/ra_types.h")
+#include "../capabilities/ra_types.h"
#else
-#include "ra_types.h"
+#error "ra_types.h not found"
#endif📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Include types from the canonical source | |
| // Note: When built as xcframework, types.h is in same directory | |
| // When building locally, it's in ../capabilities/ | |
| #if __has_include("types.h") | |
| #include "ra_types.h" | |
| #else | |
| #include "ra_types.h" | |
| #endif | |
| // Include types from the canonical source | |
| #include "ra_types.h" |
🤖 Prompt for AI Agents
In sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_llamacpp_bridge.h
around lines 32 to 39, the conditional include currently always pulls ra_types.h
and thus the __has_include check is ineffective; update the branches so the
first branch includes the local xcframework header (e.g., "types.h" or
"ra_types.h" as shipped in the framework) and the else branch includes the
local-build path (e.g., "../capabilities/types.h") so the code actually picks
the correct header depending on build environment.
sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_onnx_bridge.h
Outdated
Show resolved
Hide resolved
sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_types.h
Outdated
Show resolved
Hide resolved
| public func generateStream( | ||
| prompt: String, | ||
| config: LlamaCPPGenerationConfig = LlamaCPPGenerationConfig() | ||
| ) -> AsyncThrowingStream<String, Error> { | ||
| AsyncThrowingStream { continuation in | ||
| Task { | ||
| guard self.isReady, let backend = self.backendHandle else { | ||
| continuation.finish(throwing: LlamaCPPError.invalidHandle) | ||
| return | ||
| } | ||
|
|
||
| guard self.isModelLoaded else { | ||
| continuation.finish(throwing: LlamaCPPError.generationFailed("No model loaded")) | ||
| return | ||
| } | ||
|
|
||
| self.logger.info("Starting streaming generation for prompt: \(prompt.prefix(50))...") | ||
|
|
||
| // Create a wrapper class to pass to the C callback | ||
| class CallbackContext { | ||
| let continuation: AsyncThrowingStream<String, Error>.Continuation | ||
| var isCancelled = false | ||
|
|
||
| init(continuation: AsyncThrowingStream<String, Error>.Continuation) { | ||
| self.continuation = continuation | ||
| } | ||
| } | ||
|
|
||
| let context = CallbackContext(continuation: continuation) | ||
| let contextPtr = Unmanaged.passRetained(context).toOpaque() | ||
|
|
||
| let callback: ra_text_stream_callback = { tokenCStr, userData in | ||
| guard let userData = userData else { return false } | ||
| let context = Unmanaged<CallbackContext>.fromOpaque(userData).takeUnretainedValue() | ||
|
|
||
| if context.isCancelled { | ||
| return false | ||
| } | ||
|
|
||
| if let tokenCStr = tokenCStr { | ||
| let token = String(cString: tokenCStr) | ||
| context.continuation.yield(token) | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| let status = ra_text_generate_stream( | ||
| backend, | ||
| prompt, | ||
| config.systemPrompt, | ||
| Int32(config.maxTokens), | ||
| config.temperature, | ||
| callback, | ||
| contextPtr | ||
| ) | ||
|
|
||
| // Release the context | ||
| Unmanaged<CallbackContext>.fromOpaque(contextPtr).release() | ||
|
|
||
| if status == RA_SUCCESS || status == RA_ERROR_CANCELLED { | ||
| continuation.finish() | ||
| } else { | ||
| let errorMsg = String(cString: ra_get_last_error()) | ||
| continuation.finish(throwing: LlamaCPPError.generationFailed(errorMsg)) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for header files defining the C API
find . -type f \( -name "*.h" -o -name "*.hpp" \) | head -20Repository: RunanywhereAI/runanywhere-sdks
Length of output: 603
🏁 Script executed:
#!/bin/bash
# Look for the C API definitions
rg -n "ra_text_stream_callback|ra_text_generate_stream" --type-add 'h:*.h' --type-list | head -5Repository: RunanywhereAI/runanywhere-sdks
Length of output: 463
🏁 Script executed:
#!/bin/bash
# Search more broadly for any .h files
fd -e h -e hpp . | head -20Repository: RunanywhereAI/runanywhere-sdks
Length of output: 589
🏁 Script executed:
#!/bin/bash
# Check the file structure to understand where C bindings are
ls -la sdk/runanywhere-swift/Sources/ 2>/dev/null || ls -la . | head -20Repository: RunanywhereAI/runanywhere-sdks
Length of output: 645
🏁 Script executed:
#!/bin/bash
# Check the LlamaCPP bridge header for callback definitions
cat sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_llamacpp_bridge.hRepository: RunanywhereAI/runanywhere-sdks
Length of output: 18112
🏁 Script executed:
#!/bin/bash
# Check the core types
cat sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_types.hRepository: RunanywhereAI/runanywhere-sdks
Length of output: 2718
🏁 Script executed:
#!/bin/bash
# Look for other streaming implementations
rg -n "AsyncThrowingStream" --type swift sdk/runanywhere-swift/Repository: RunanywhereAI/runanywhere-sdks
Length of output: 7135
🏁 Script executed:
#!/bin/bash
# Search for Task or threading patterns in LlamaCPPService
rg -n "Task|DispatchQueue|Thread|NSLock" --type swift sdk/runanywhere-swift/Sources/LlamaCPPRuntime/ -B2 -A2Repository: RunanywhereAI/runanywhere-sdks
Length of output: 699
🏁 Script executed:
#!/bin/bash
# Check if there's Actor or Sendable patterns
rg -n "actor|@Sendable|nonisolated" --type swift sdk/runanywhere-swift/Repository: RunanywhereAI/runanywhere-sdks
Length of output: 15456
🏁 Script executed:
#!/bin/bash
# Look at the full generateStream function and surrounding context
sed -n '200,285p' sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swiftRepository: RunanywhereAI/runanywhere-sdks
Length of output: 3306
🏁 Script executed:
#!/bin/bash
# Search for calls to cancel() method and any mutation of isCancelled
rg -n "\.cancel\(\)|isCancelled" --type swift sdk/runanywhere-swift/Sources/LlamaCPPRuntime/Repository: RunanywhereAI/runanywhere-sdks
Length of output: 306
🏁 Script executed:
#!/bin/bash
# Check if there's more context in the file about how cancellation works
cat sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift | head -50Repository: RunanywhereAI/runanywhere-sdks
Length of output: 1704
🏁 Script executed:
#!/bin/bash
# Search for test files to understand expected behavior
find . -name "*Test*.swift" -o -name "*test*.swift" | grep -i llama | head -5Repository: RunanywhereAI/runanywhere-sdks
Length of output: 56
🌐 Web query:
AsyncThrowingStream Continuation.yield thread safety Swift 6 concurrency
💡 Result:
Short answer: Yes — AsyncThrowingStream.Continuation.yield is designed to be safe to call from concurrent threads/tasks. The Continuation type is Sendable and its operations (yield/finish/finish(throwing:)) are serialized internally, so you can call yield from multiple threads. However, concurrent yields can produce out-of-order delivery relative to the callers, and AsyncStream/AsyncThrowingStream do not provide blocking backpressure (use an explicit actor/lock or a backpressure-aware API if you need strict ordering or producer-side suspension). [1][2][3]
Sources:
- AsyncStream/AsyncThrowingStream pitch (notes that continuation is usable from concurrent contexts; calls are serialized; out-of-order delivery possible). [1]
- Apple API docs for AsyncStream/AsyncThrowingStream (Continuation is Sendable / Continuation API). [2]
- Swift Forums discussions explaining escaping/sendable nature and ordering/backpressure caveats. [3]
🏁 Script executed:
#!/bin/bash
# Check the full LlamaCPPService class structure to understand lifecycle
wc -l sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift
head -150 sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swiftRepository: RunanywhereAI/runanywhere-sdks
Length of output: 4846
🏁 Script executed:
#!/bin/bash
# Search for any other streaming implementations to see if isCancelled pattern is used elsewhere
rg -n "isCancelled|CallbackContext" --type swift sdk/runanywhere-swift/Repository: RunanywhereAI/runanywhere-sdks
Length of output: 1136
🏁 Script executed:
#!/bin/bash
# Check if there's a way to associate context with backend for cancellation
rg -n "backendHandle|context" --type swift sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift | head -30Repository: RunanywhereAI/runanywhere-sdks
Length of output: 1444
Fix thread safety issues and implement proper cancellation in generateStream method.
The callback context pattern has multiple issues that violate Swift 6 concurrency requirements:
-
Dead code: The
isCancelledflag is initialized but never set totrue, making the cancellation check on line 246 unreachable. -
Swift 6 non-compliance:
CallbackContextis notSendableor anactor, but it's accessed from a C callback thread without proper isolation. Per the coding guidelines, Swift 6 concurrency primitives must be used. -
Incomplete cancellation: The
cancel()method callsra_text_cancel()but doesn't notify the callback context, so the streaming callback has no way to be cancelled from the Swift side.
Recommended fix:
- Make
CallbackContextan actor or mark itSendablewith proper thread safety - Use
Task.isCancelledpattern (already used elsewhere in the codebase) instead of a custom flag - Ensure the callback properly checks for cancellation if the C library supports it
- Consider storing the streaming context in a thread-safe manner to enable proper cancellation integration
| public enum ConfigurationDefaults { | ||
| // Generation settings | ||
| public static let temperature: Double = 0.7 | ||
| public static let temperature: Double = 0.8 // Match LLM.swift default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the stale comment reference.
The comment references "LLM.swift" which is being replaced by LlamaCPP in this PR. Update the comment to reflect the actual rationale for the 0.8 default temperature or reference the correct library.
Apply this diff:
- public static let temperature: Double = 0.8 // Match LLM.swift default
+ public static let temperature: Double = 0.8 // Match LlamaCPP default📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static let temperature: Double = 0.8 // Match LLM.swift default | |
| public static let temperature: Double = 0.8 // Match LlamaCPP default |
🤖 Prompt for AI Agents
In
sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Constants/SDKConstants.swift
around line 68, update the stale comment that references "LLM.swift" to
correctly state the rationale for the default 0.8 temperature (e.g., "Default
temperature tuned for balanced creativity and determinism" or reference the
current library LlamaCPP if that informed the choice); replace the old reference
with the accurate explanation or correct library name so the comment reflects
why 0.8 was chosen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
68 files reviewed, no comments
…tion - Enhanced the JNI module to provide clearer logging for library download failures and added a mechanism to create an empty version file when libraries are not available. - Updated build.gradle.kts files for LlamaCPP and ONNX modules to reference the JNI module directly, simplifying project structure. - Introduced remote distribution of the unified RunAnywhereCore.xcframework, eliminating the need for local builds and ensuring consistent binary versions across SDK consumers. - Updated README and documentation to reflect the new binary distribution approach and its benefits. This commit improves the SDK's architecture by streamlining library management and enhancing the user experience with remote binary distribution.
…ere SDK - Introduced a new property `runanywhere.testLocal` in gradle.properties to toggle local library usage for the RunAnywhere SDK. - Updated build.gradle.kts files across JNI, LlamaCPP, and ONNX modules to reference the new `testLocal` property for local build configurations. - Removed deprecated `.version` files from JNI and LlamaCPP modules to streamline project structure. - Enhanced documentation in README and .gitignore to clarify local vs. remote XCFramework distribution settings. This commit improves the SDK's flexibility by allowing developers to easily switch between local and remote library usage, enhancing the development experience.
- Added logic to ensure the destination directory exists before downloading files. - Moved file handling operations inside the download task callback to prevent issues with temporary file existence. - Enhanced error handling during file moves to ensure robustness in the download process. This commit enhances the reliability of file downloads in the ONNXDownloadStrategy, ensuring that files are correctly managed and potential errors are handled gracefully.
- Introduced a new binary target for RunAnywhereCore in Package.swift, enabling easier integration of the C++ core library. - Updated the RunAnywhere target to include the new RunAnywhereCore dependency, enhancing modularity. - Revised comments in settings.gradle.kts and build.gradle.kts files to clarify the structure and self-contained nature of backend modules. - Adjusted JNI library handling in build.gradle.kts to ensure proper resolution of native dependencies. This commit improves the SDK's architecture by streamlining the integration of the RunAnywhereCore library and clarifying module dependencies.
- Changed `runanywhere.testLocal` property in gradle.properties from true to false to default to remote library usage. - Updated version numbers for RunAnywhere Core LlamaCPP and ONNX modules to 0.0.1-dev.6ab47eb. - Added a new `.version` file for the LlamaCPP JNI libraries to maintain version consistency. This commit ensures the SDK defaults to using pre-built libraries and maintains updated versioning across modules.
|
@greptile please review |
|
@sanchitmonga22: I'll provide a comprehensive review of this significant architectural change. High-Level OverviewThis PR represents a major refactoring that replaces the LLM.swift dependency with a unified RunAnywhereLlamaCPP backend powered by the runanywhere-core library. The scope is substantial, touching both iOS and Android platforms. Key Architectural Changes✅ Strengths:
Specific ObservationsAndroid (Kotlin):
iOS (Swift):
Build System:
Since this is a draft PR with significant changes, I'll trigger a comprehensive review to provide detailed feedback on each component. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 5181607 in 3 minutes and 53 seconds. Click for details.
- Reviewed
8360lines of code in73files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:47
- Draft comment:
Ensure consistent indentation in the initializer. It appears that the parameter 'enableRealTimeTracking' might be misaligned compared to others. - Reason this comment was not posted:
Comment was on unchanged code.
2. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:36
- Draft comment:
Default values (e.g. temperature = 0.8, maxTokens = 100) are clear and match LLM.swift defaults; consider documenting units and expected range for parameters like temperature and topP to benefit future maintainers. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:1
- Draft comment:
Good use of 'Sendable' conformance for thread safety in a value type; this practice improves concurrent usage of generation options. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. README.md:131
- Draft comment:
There's an unexpected standalone ')' on line 131. If this character is not meant to be there, please remove it. - Reason this comment was not posted:
Comment was on unchanged code.
5. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:40
- Draft comment:
Typographical error: There's an extra '/' at the beginning of the comment for 'enableRealTimeTracking'. It should be formatted as "/// - enableRealTimeTracking: Enable real-time cost tracking (default: true)" without the additional '/'. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_coQd8VkpQlBVLSUn
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swift (1)
209-209: Avoid force unwrapping URL construction.Appending ".json" to a URL string should always produce a valid URL, but force unwrapping is not defensive.
Apply this diff:
- let configURL = URL(string: downloadURL.absoluteString + ".json")! + guard let configURL = URL(string: downloadURL.absoluteString + ".json") else { + throw DownloadError.invalidURL + }
♻️ Duplicate comments (5)
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.kts (1)
238-255:⚠️ Timeout and checksum verification missing from download task.This is the same timeout and security concern flagged in a prior review. The
URL().openStream()call at line 240 has no connection or read timeouts, risking indefinite build hangs on network issues. Additionally, there's no checksum verification of the downloaded binary, creating a supply chain risk.Recommend factoring the download logic into a shared Gradle build convention plugin used by both this module and the LlamaCPP module to avoid duplication and ensure consistent security practices across all backends.
Consider applying this pattern:
+import java.net.HttpURLConnection + // Download the ZIP file try { logger.lifecycle("Downloading from: $downloadUrl") - URL(downloadUrl).openStream().use { input -> + val connection = URL(downloadUrl).openConnection() as HttpURLConnection + connection.connectTimeout = 30_000 // 30 seconds + connection.readTimeout = 120_000 // 2 minutes for large files + connection.inputStream.use { input -> zipFile.outputStream().use { output -> input.copyTo(output) } } logger.lifecycle("Downloaded: ${zipFile.length() / 1024}KB") + // Verify SHA256 checksum before proceedingsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts (1)
238-255: Add timeout and checksum verification for downloaded native binaries.The
URL().openStream()call at line 240 has no connection or read timeouts, creating a risk of indefinite build hangs on network issues. Additionally, downloading binaries without checksum verification poses a supply chain security risk.Implement connection timeouts and checksum verification:
+import java.net.HttpURLConnection + // Download the ZIP file try { logger.lifecycle("Downloading from: $downloadUrl") - URL(downloadUrl).openStream().use { input -> + val connection = URL(downloadUrl).openConnection() as HttpURLConnection + connection.connectTimeout = 30_000 // 30 seconds + connection.readTimeout = 120_000 // 2 minutes for large files + connection.inputStream.use { input -> zipFile.outputStream().use { output -> input.copyTo(output) } } logger.lifecycle("Downloaded: ${zipFile.length() / 1024}KB") + // TODO: Verify SHA256 checksum before proceeding } catch (e: Exception) {To avoid duplication, extract this download logic and the ONNX module's download logic into a shared Gradle convention plugin applied by both backend modules.
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt (2)
144-149: Race condition indestroy()- missing mutex.This issue was previously flagged. The
destroy()method modifiesbackendHandlewithout acquiring the mutex, unlikeinitialize(),loadModel(), andgenerate(). This can cause use-after-free ifdestroy()is called while another operation is in progress.-fun destroy() { - if (backendHandle != 0L) { - RunAnywhereBridge.nativeDestroy(backendHandle) - backendHandle = 0 +suspend fun destroy() { + withContext(Dispatchers.IO) { + mutex.withLock { + if (backendHandle != 0L) { + RunAnywhereBridge.nativeDestroy(backendHandle) + backendHandle = 0L + } + } } }
312-327: IncorrectcallbackFlowusage -close()called unconditionally after try-catch.This issue was previously flagged. The
close()call on line 325 is outside the try-catch, meaning it's called even afterclose(e)in the catch block. Moveclose()inside the try block to signal normal completion only on success.fun generateFlow( prompt: String, systemPrompt: String? = null, maxTokens: Int = 256, temperature: Float = 0.8f ): Flow<String> = callbackFlow { try { generateStream(prompt, systemPrompt, maxTokens, temperature) { token -> trySend(token).isSuccess } + close() // Signal completion after streaming finishes } catch (e: Exception) { close(e) } - close() awaitClose { cancel() } }sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swift (1)
358-358: Silent error suppression on archive path (same pattern as line 272).Using
try?to remove the existing archive silently ignores errors. While this is less critical than indownloadFile(since it's a temp file), explicit checks improve debuggability.
🧹 Nitpick comments (12)
sdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.kt (2)
91-97: Silently swallowingUnsatisfiedLinkErrormay mask genuine library loading failures.While the intent is to handle already-loaded libraries gracefully, this approach also hides legitimate failures (e.g., missing library, ABI mismatch). Consider logging the exception at debug level to aid troubleshooting without breaking the flow.
private fun tryLoadLibrary(name: String) { try { System.loadLibrary(name) } catch (e: UnsatisfiedLinkError) { - // May already be loaded or linked statically - this is fine + // May already be loaded or linked statically - log for debugging + // Log.d("RunAnywhereBridge", "Library $name may already be loaded: ${e.message}") } }
69-86: Backend library names are hard-coded; consider making this extensible.The
whenblock hard-codes library names for each backend. Based on learnings, the codebase prefers theModuleRegistryprovider pattern for extensibility. Consider allowing backends to register their required libraries dynamically, which would simplify adding new backends without modifying this bridge.sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt (2)
253-273: JSON parsing uses regex instead of a proper JSON parser.The manual regex-based JSON extraction is fragile and may fail on edge cases (e.g., nested objects, Unicode escapes like
\uXXXX). Consider usingkotlinx.serializationor another JSON library for reliable parsing. The fallback to raw response is a reasonable safeguard, but could hide parsing bugs.// Example using kotlinx.serialization @Serializable data class GenerationResponse(val text: String, val finish_reason: String? = null) private fun parseTextFromResponse(jsonResponse: String): String { return try { Json.decodeFromString<GenerationResponse>(jsonResponse).text } catch (e: Exception) { jsonResponse // fallback } }
48-52: Init block performs blocking I/O during object construction.Loading native libraries in the
initblock means construction blocks until libraries are loaded. This could cause ANR issues if called on the main thread. Consider using lazy initialization or requiring explicitloadLibrary()calls before use, consistent with the coding guidelines preference for lazy initialization.class LlamaCppCoreService { private var backendHandle: Long = 0 private val mutex = Mutex() + private var librariesLoaded = false - init { - // Load JNI bridge and LlamaCPP backend libraries on construction - RunAnywhereBridge.loadLibrary() - RunAnywhereBridge.loadBackend("llamacpp") - } + private fun ensureLibrariesLoaded() { + if (!librariesLoaded) { + RunAnywhereBridge.loadLibrary() + RunAnywhereBridge.loadBackend("llamacpp") + librariesLoaded = true + } + }Then call
ensureLibrariesLoaded()at the start ofinitialize().sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swift (3)
242-283: Consider Swift 6 async URLSession APIs and add timeout configuration.The current implementation uses
CheckedContinuationto bridge completion-handler-basedURLSession.downloadTaskto async/await. Swift 5.5+ provides native async methods onURLSessionthat eliminate the need for manual continuation handling, and you should configure a timeout to prevent indefinitely hung downloads.As per coding guidelines, use the latest Swift 6 APIs. Here's a modern refactor:
private func downloadFile(from url: URL, to destination: URL, progressHandler: ((Double) -> Void)?) async throws { logger.debug("Downloading file from: \(url.absoluteString)") // Ensure destination directory exists let destinationDir = destination.deletingLastPathComponent() try FileManager.default.createDirectory(at: destinationDir, withIntermediateDirectories: true) // Configure URLSession with timeout let config = URLSessionConfiguration.default config.timeoutIntervalForRequest = 60 config.timeoutIntervalForResource = 300 let session = URLSession(configuration: config) // Use native async download (Swift 5.5+) let (downloadedURL, response) = try await session.download(from: url) guard let httpResponse = response as? HTTPURLResponse, (200...299).contains(httpResponse.statusCode) else { throw DownloadError.invalidResponse } // Move file to destination try? FileManager.default.removeItem(at: destination) try FileManager.default.moveItem(at: downloadedURL, to: destination) progressHandler?(1.0) }
272-273: Silent error suppression could obscure permission or directory conflicts.Using
try?to remove an existing item before moving will silently ignore errors. Ifdestinationexists as a directory or has permission issues,removeItemfails silently and the subsequentmoveItemfails with an unclear error.Consider explicit error handling:
- try? FileManager.default.removeItem(at: destination) - try FileManager.default.moveItem(at: downloadedURL, to: destination) + // Remove existing file if present + if FileManager.default.fileExists(atPath: destination.path) { + try FileManager.default.removeItem(at: destination) + } + try FileManager.default.moveItem(at: downloadedURL, to: destination)
336-366: Same continuation pattern asdownloadFile– consider consolidating.This segment uses the same manual
CheckedContinuationpattern asdownloadFile(lines 242-283) and inherits the same concerns: no timeout configuration and verbose completion-handler bridging. The earlier refactor suggestion to use Swift's native asyncURLSession.download(from:)applies here as well.sdk/runanywhere-swift/README.md (1)
50-51: Consider linking to binary distribution detailsThe high-level binary distribution note is clear. It may help readers to add a short pointer to
sdk/runanywhere-swift/Binaries/README.mdfor local XCFramework/dev-mode instructions.sdk/runanywhere-swift/Binaries/README.md (1)
55-55: Fix markdownlint bare URL warningLine 55 uses a bare URL. To satisfy MD034 and keep formatting consistent, consider changing it to something like:
1. Find the latest release at: [runanywhere-binaries releases](https://github.com/RunanywhereAI/runanywhere-binaries/releases)sdk/runanywhere-swift/Package.swift (1)
4-11: Binary target selection logic is solid and fixes the local-path issueThe
testLocalflag plusbinaryTargets()helper cleanly separates local vs remote XCFramework usage, with remote GitHub release as the default. This addresses the earlier problem of depending on a non-existent local path and keeps the manifest production-friendly. Just be careful not to committestLocal = trueunintentionally.Also applies to: 208-209, 215-236
sdk/runanywhere-swift/ARCHITECTURE.md (2)
73-81: Tighten up CRunAnywhereCore block (language + headers)
- The directory tree fenced block at line 73 has no language; specifying something like
```textwill satisfy MD040 and keep markdownlint happy.- Given the header list now includes
ra_llamacpp_bridge.handra_core.h, consider updating themodule.modulemapexample to export all relevant headers, not justra_types.handra_onnx_bridge.h, so the docs accurately reflect the LlamaCPP bridge.Also applies to: 85-90
475-491: “Future: Adding llama.cpp to Core” section may now be staleEarlier in this doc you state that
RunAnywhereCore.xcframeworkalready includes both ONNX Runtime and LlamaCPP backends. The “Future: Adding llama.cpp to Core” section still describes this as a future plan, which can confuse readers. Consider either removing or rephrasing this section to describe the current layout of the LlamaCPP backend insiderunanywhere-core.Also applies to: 503-520
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
Package.swift(1 hunks)examples/android/RunAnywhereAI/app/build.gradle.kts(2 hunks)examples/android/RunAnywhereAI/settings.gradle.kts(1 hunks)examples/ios/RunAnywhereAI/RunAnywhereAI.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved(4 hunks)sdk/runanywhere-kotlin/gradle.properties(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-jni/VERSION(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.kts(4 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.kt(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/VERSION(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/main/jniLibs/.version(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/VERSION(1 hunks)sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.kts(6 hunks)sdk/runanywhere-kotlin/settings.gradle.kts(1 hunks)sdk/runanywhere-swift/.gitignore(1 hunks)sdk/runanywhere-swift/ARCHITECTURE.md(6 hunks)sdk/runanywhere-swift/Binaries/README.md(1 hunks)sdk/runanywhere-swift/Package.swift(6 hunks)sdk/runanywhere-swift/README.md(7 hunks)sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swift(6 hunks)
✅ Files skipped from review due to trivial changes (4)
- sdk/runanywhere-kotlin/modules/runanywhere-core-jni/VERSION
- sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/VERSION
- sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/main/jniLibs/.version
- sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/VERSION
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/android/RunAnywhereAI/app/build.gradle.kts
- sdk/runanywhere-swift/.gitignore
- examples/ios/RunAnywhereAI/RunAnywhereAI.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
🧰 Additional context used
📓 Path-based instructions (4)
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Use the latest Swift 6 APIs always.
Do not use NSLock as it is outdated. Use Swift 6 concurrency primitives instead.
Files:
Package.swiftsdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swiftsdk/runanywhere-swift/Package.swift
sdk/runanywhere-kotlin/**/src/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
sdk/runanywhere-kotlin/**/src/**/*.kt: All business logic, protocols, interfaces, and structures MUST be defined incommonMain/for Kotlin Multiplatform projects. Platform-specific implementations must use clear prefixes likeAndroidTTSService.kt,JvmTTSService.kt, etc.
NEVER make assumptions when implementing KMP code. ALWAYS refer to the iOS implementation as the definitive source of truth. Check the iOS implementation, copy the logic exactly, and adapt only for Kotlin syntax.
Useexpect/actualONLY for platform-specific implementations, not for business logic. Put all business logic incommonMain/.
Use Flow instead of AsyncSequence for reactive streams in Kotlin Multiplatform code.
Use coroutines and suspend functions instead of async/await patterns for asynchronous operations.
Use sealed classes for type-safe error handling in Kotlin code.
Always use data classes and structured types (enums, sealed classes) instead of strings for models and configuration.
Use theModuleRegistryprovider pattern for extensibility and plugin architecture instead of hard-coded dependencies.
Use EventBus for component communication instead of direct coupling between components.
Use Service Container pattern for centralized dependency injection with lazy initialization.
Use lazy initialization for service dependencies to avoid memory pressure and allow garbage collection.
Files:
sdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt
sdk/runanywhere-kotlin/**/*.gradle.kts
📄 CodeRabbit inference engine (CLAUDE.md)
sdk/runanywhere-kotlin/**/*.gradle.kts: Kotlin version must be 2.1.21 or later for the Kotlin Multiplatform SDK.
JVM target must be 17 or later for Kotlin Multiplatform SDK.
Android minSdk must be 24 or later for Kotlin Multiplatform SDK.
Files:
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.kts
sdk/runanywhere-swift/**/Package.swift
📄 CodeRabbit inference engine (CLAUDE.md)
sdk/runanywhere-swift/**/Package.swift: iOS minimum deployment target must be 13.0 or later for Swift SDK.
Swift minimum version must be 5.9 or later for iOS SDK.
Files:
sdk/runanywhere-swift/Package.swift
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use the `ModuleRegistry` provider pattern for extensibility and plugin architecture instead of hard-coded dependencies.
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : All business logic, protocols, interfaces, and structures MUST be defined in `commonMain/` for Kotlin Multiplatform projects. Platform-specific implementations must use clear prefixes like `AndroidTTSService.kt`, `JvmTTSService.kt`, etc.
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/androidMain/**/*.kt : Use platform-specific optimizations in their respective source directories: `androidMain/` for Android optimizations, `jvmMain/` for JVM optimizations.
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/components/**/*.kt : Implement proper component lifecycle management with initialization, state tracking, and cleanup in BaseComponent.
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use lazy initialization for service dependencies to avoid memory pressure and allow garbage collection.
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-swift/**/Package.swift : Swift minimum version must be 5.9 or later for iOS SDK.
Applied to files:
Package.swiftsdk/runanywhere-swift/README.mdsdk/runanywhere-swift/ARCHITECTURE.mdsdk/runanywhere-swift/Package.swift
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-swift/**/Package.swift : iOS minimum deployment target must be 13.0 or later for Swift SDK.
Applied to files:
Package.swiftsdk/runanywhere-swift/README.mdsdk/runanywhere-swift/ARCHITECTURE.mdsdk/runanywhere-swift/Package.swift
📚 Learning: 2025-10-17T22:19:36.568Z
Learnt from: shubhammalhotra28
Repo: RunanywhereAI/runanywhere-sdks PR: 119
File: README.md:176-176
Timestamp: 2025-10-17T22:19:36.568Z
Learning: The RunAnywhere SDK package repository is located at `https://github.com/RunanywhereAI/runanywhere-sdks`. The repository was moved from `sdks` to `runanywhere-sdks`, so the correct URL to reference in documentation and installation instructions is `https://github.com/RunanywhereAI/runanywhere-sdks`.
Applied to files:
Package.swiftsdk/runanywhere-swift/README.mdsdk/runanywhere-swift/ARCHITECTURE.mdexamples/android/RunAnywhereAI/settings.gradle.ktssdk/runanywhere-swift/Binaries/README.md
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/androidMain/**/*.kt : Use platform-specific optimizations in their respective source directories: `androidMain/` for Android optimizations, `jvmMain/` for JVM optimizations.
Applied to files:
sdk/runanywhere-kotlin/gradle.propertiessdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/jvmMain/**/*.kt : Use platform-specific optimizations in their respective source directories: `androidMain/` for Android optimizations, `jvmMain/` for JVM optimizations.
Applied to files:
sdk/runanywhere-kotlin/gradle.propertiessdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/*.gradle.kts : Kotlin version must be 2.1.21 or later for the Kotlin Multiplatform SDK.
Applied to files:
sdk/runanywhere-kotlin/gradle.propertiessdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use the `ModuleRegistry` provider pattern for extensibility and plugin architecture instead of hard-coded dependencies.
Applied to files:
sdk/runanywhere-kotlin/gradle.propertiessdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/*.gradle.kts : Android minSdk must be 24 or later for Kotlin Multiplatform SDK.
Applied to files:
sdk/runanywhere-kotlin/gradle.propertiessdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/*.gradle.kts : JVM target must be 17 or later for Kotlin Multiplatform SDK.
Applied to files:
sdk/runanywhere-kotlin/gradle.propertiessdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use lazy initialization for service dependencies to avoid memory pressure and allow garbage collection.
Applied to files:
sdk/runanywhere-kotlin/gradle.propertiessdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.ktsdk/runanywhere-kotlin/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use `expect/actual` ONLY for platform-specific implementations, not for business logic. Put all business logic in `commonMain/`.
Applied to files:
sdk/runanywhere-kotlin/gradle.propertiessdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use coroutines and suspend functions instead of async/await patterns for asynchronous operations.
Applied to files:
sdk/runanywhere-kotlin/gradle.propertiessdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : NEVER make assumptions when implementing KMP code. ALWAYS refer to the iOS implementation as the definitive source of truth. Check the iOS implementation, copy the logic exactly, and adapt only for Kotlin syntax.
Applied to files:
sdk/runanywhere-kotlin/gradle.properties
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Always use data classes and structured types (enums, sealed classes) instead of strings for models and configuration.
Applied to files:
sdk/runanywhere-kotlin/gradle.properties
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : All business logic, protocols, interfaces, and structures MUST be defined in `commonMain/` for Kotlin Multiplatform projects. Platform-specific implementations must use clear prefixes like `AndroidTTSService.kt`, `JvmTTSService.kt`, etc.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.ktsdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.ktssdk/runanywhere-kotlin/settings.gradle.ktsexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use Service Container pattern for centralized dependency injection with lazy initialization.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.ktssdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.ktexamples/android/RunAnywhereAI/settings.gradle.kts
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Applies to sdk/runanywhere-kotlin/**/src/**/*.kt : Use Flow instead of AsyncSequence for reactive streams in Kotlin Multiplatform code.
Applied to files:
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt
📚 Learning: 2025-11-26T22:49:40.358Z
Learnt from: CR
Repo: RunanywhereAI/runanywhere-sdks PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T22:49:40.358Z
Learning: Use `./scripts/sdk.sh` for all Kotlin SDK operations instead of calling Gradle directly. It handles configuration and build complexity.
Applied to files:
examples/android/RunAnywhereAI/settings.gradle.kts
🧬 Code graph analysis (2)
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts (1)
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.kts (1)
resolveModulePath(63-76)
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.kts (1)
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts (1)
resolveModulePath(63-76)
🪛 detekt (1.23.8)
sdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.kt
[warning] 94-94: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt
[warning] 269-269: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🪛 markdownlint-cli2 (0.18.1)
sdk/runanywhere-swift/ARCHITECTURE.md
73-73: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
sdk/runanywhere-swift/Binaries/README.md
55-55: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (15)
sdk/runanywhere-kotlin/gradle.properties (1)
17-20: Well-structured native library configuration property.The new
runanywhere.testLocalproperty is clearly documented and follows Gradle conventions. The default value (false) sensibly uses pre-built binaries for production workflows while allowing local testing via override. The property is properly integrated into the build system across multiple modules (runanywhere-core-onnx and runanywhere-core-llamacpp), usingproject.findProperty("runanywhere.testLocal")?.toString()?.toBoolean()with a safe fallback to the default value.sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/build.gradle.kts (2)
63-76: Module path resolution supports flexible composite builds.The
resolveModulePath()function correctly handles both direct SDK builds and example app composite builds, trying multiple path prefixes before defaulting (lines 65-75). This is sound architecture for cross-module resolution.
115-119: Correct: JNI bridge dependency added to jvmAndroidMain.The API dependency on the shared JNI bridge module (line 117) is properly scoped to
jvmAndroidMain, making it available to both JVM and Android implementations as intended.sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts (2)
113-119: JNI bridge dependency properly scoped to jvmAndroidMain.The API dependency on
runanywhere-core-jniis correctly placed in thejvmAndroidMainsource set, ensuring the shared JNI bridge is available for both JVM and Android implementations (line 117).
82-143: Kotlin Multiplatform and Android configuration meets all version requirements.JVM target is set to 17 (lines 85, 91), Android minSdk is 24 (line 143), and Kotlin plugin version is 2.1.21 (gradle/libs.versions.toml), all meeting SDK constraints. Android namespace is correctly configured for the backend (line 139).
examples/android/RunAnywhereAI/settings.gradle.kts (1)
38-52: Module wiring correctly updates to new core backends.The transition from legacy
runanywhere-llm-llamacppto new self-contained backend modules is cleanly implemented. All three module paths are correctly resolved relative to the example app directory:
- JNI bridge (shared Kotlin code):
../../../sdk/runanywhere-kotlin/modules/runanywhere-core-jni- LlamaCPP backend (~45MB):
../../../sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp- ONNX backend (~30MB):
../../../sdk/runanywhere-kotlin/modules/runanywhere-core-onnxThe comments clearly document that each backend is self-contained with its native libraries, aiding developer understanding.
sdk/runanywhere-kotlin/settings.gradle.kts (1)
37-55: Module structure clearly organized with helpful documentation.The new module wiring cleanly separates concerns:
- JNI bridge (line 45): Shared Kotlin JNI declarations used by all backends
- Backend modules (lines 50, 55): Self-contained with all native libraries bundled
The comments clearly explain each module's purpose, size, and usage pattern, making it easy for developers to understand which backend to depend on.
sdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.kts (1)
34-93: Kotlin Multiplatform configuration meets all version requirements.JVM target is correctly set to 17 (lines 37, 43), Android minSdk is 24 (line 91), and Kotlin version is 2.1.21 (from gradle/libs.versions.toml), all meeting SDK constraints. The namespace correctly uses
com.runanywhere.sdk.jni.bridge(line 87) to avoid the Java keywordnative.sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt (1)
275-301:generateStreamdoesn't honor the callback return value for cancellation.The
onTokencallback returnsBooleanto signal cancellation, but the current implementation ignores it since the full response is emitted as a single token. Once native streaming is implemented, ensure the return value is checked to stop generation.The TODO on line 299 acknowledges this. Ensure follow-up work tracks implementing proper streaming support.
sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swift (2)
289-314: LGTM!The native archive extraction logic properly handles C bridge interop with
withCString, creates the destination directory, and has comprehensive error handling for all result codes fromra_extract_archive.
3-3: No action needed. TheCRunAnywhereCorebridge correctly exportsra_extract_archiveas declared in bothra_onnx_bridge.handra_llamacpp_bridge.h. The import change at line 3 is valid and consistent with other files in the codebase.Package.swift (1)
44-49: RunAnywhereCore binary target wiring looks goodRemote XCFramework target and its use in the
RunAnywheretarget are consistent with the Swift SDK manifest; no functional issues here. Just keep this URL/checksum in sync withsdk/runanywhere-swift/Package.swiftwhen bumping core binaries.Also applies to: 55-55
sdk/runanywhere-swift/README.md (2)
117-133: LlamaCPP import and registration flow align with new adapter/service
import LlamaCPPRuntime,LlamaCPPServiceProvider.register(), andLlamaCPPCoreAdapter()are consistent with the new LlamaCPP backend architecture and ModuleRegistry pattern. The snippet reads correctly and matches the Package.swift product/target names.
329-336: GGUF + acknowledgments updates are consistentReferencing GGUF models “via llama.cpp” and explicitly crediting “The llama.cpp community” matches the new backend and helps clarify responsibilities; no changes needed.
Also applies to: 472-472
sdk/runanywhere-swift/Package.swift (1)
45-51: New public products and backend targets are consistent
RunAnywhereLlamaCPP→LlamaCPPRuntimeandRunAnywhereWhisperKit→WhisperKitTranscriptionproducts match their respective targets, and ONNX/Llama/FluidAudio targets all depend on the expected core modules. This public surface aligns with the README and architecture docs.Also applies to: 55-59, 171-207
...re-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.kt
Show resolved
Hide resolved
| fun resolveModulePath(moduleName: String): String { | ||
| // Try to find the module with different path prefixes | ||
| val possiblePaths = listOf( | ||
| ":modules:$moduleName", // When building SDK directly | ||
| ":sdk:runanywhere-kotlin:modules:$moduleName", // When included from example app | ||
| ) | ||
| for (path in possiblePaths) { | ||
| if (project.findProject(path) != null) { | ||
| return path | ||
| } | ||
| } | ||
| // Default to the most common path | ||
| return ":modules:$moduleName" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Extract resolveModulePath to shared build logic.
This function is duplicated across both runanywhere-core-onnx and runanywhere-core-llamacpp modules. Extract it to a shared Gradle plugin or convention to avoid maintenance burden and ensure both backends handle composite builds consistently.
Consider creating sdk/runanywhere-kotlin/build-logic/src/main/kotlin/ModulePathResolver.kt or a similar shared conventions plugin:
// Shared build convention
fun Project.resolveModulePath(moduleName: String): String {
val possiblePaths = listOf(
":modules:$moduleName",
":sdk:runanywhere-kotlin:modules:$moduleName",
)
for (path in possiblePaths) {
if (findProject(path) != null) return path
}
return ":modules:$moduleName"
}Then apply it in both build.gradle.kts files.
🤖 Prompt for AI Agents
sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/build.gradle.kts lines
63-76: the resolveModulePath function is duplicated across runanywhere-core-onnx
and runanywhere-core-llamacpp; extract it into a shared build-logic module (e.g.
sdk/runanywhere-kotlin/build-logic/src/main/kotlin/ModulePathResolver.kt) as an
extension function on Project (fun Project.resolveModulePath(moduleName:
String): String { ... }), publish/apply that convention/plugin from
sdk/runanywhere-kotlin/build-logic in both modules' build.gradle.kts, replace
the local function usages with project.resolveModulePath(...) in both build
files, and remove the duplicated function implementations so both backends use
the single shared resolver.
...-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt
Outdated
Show resolved
Hide resolved
| // ================================================================= | ||
| // C Bridge Module - Exposes unified xcframework C APIs to Swift | ||
| // ================================================================= | ||
| .target( | ||
| name: "CRunAnywhereCore", | ||
| dependencies: ["RunAnywhereCoreBinary"], | ||
| path: "Sources/CRunAnywhereCore", | ||
| publicHeadersPath: "include" | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check that CRunAnywhereCore headers and module.modulemap include both ONNX and LlamaCPP bridges
echo "=== CRunAnywhereCore headers ==="
fd -e h . sdk/runanywhere-swift/Sources/CRunAnywhereCore --max-depth 3 || true
echo ""
echo "=== module.modulemap contents ==="
fd module.modulemap sdk/runanywhere-swift/Sources/CRunAnywhereCore -x cat {} || echo "module.modulemap not found"
echo ""
echo "=== Directory structure ==="
find sdk/runanywhere-swift/Sources/CRunAnywhereCore -type f | head -20Repository: RunanywhereAI/runanywhere-sdks
Length of output: 950
Add missing headers to CRunAnywhereCore modulemap
The module.modulemap currently exports only ra_types.h and ra_onnx_bridge.h, but the include directory also contains ra_core.h and ra_llamacpp_bridge.h. Update the modulemap to explicitly include all bridge headers so both ONNX and LlamaCPP backends are fully accessible to Swift consumers:
module CRunAnywhereCore {
header "ra_types.h"
header "ra_core.h"
header "ra_onnx_bridge.h"
header "ra_llamacpp_bridge.h"
export *
}
🤖 Prompt for AI Agents
In sdk/runanywhere-swift/Package.swift around lines 119 to 127, the
CRunAnywhereCore target's module.modulemap currently only exports ra_types.h and
ra_onnx_bridge.h while the include directory also contains ra_core.h and
ra_llamacpp_bridge.h; update the module.modulemap to explicitly add header
entries for "ra_core.h" and "ra_llamacpp_bridge.h" alongside the existing
headers (ra_types.h and ra_onnx_bridge.h) so the module exports all bridge
headers and both ONNX and LlamaCPP backends are accessible to Swift consumers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 5181607 in 3 minutes and 36 seconds. Click for details.
- Reviewed
8360lines of code in73files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:51
- Draft comment:
Minor: Parameter alignment is inconsistent forenableRealTimeTrackingparameter. Consider indenting uniformly for better readability. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:39
- Draft comment:
Good: The default temperature of 0.8 aligns with the LLM.swift default; this update appears intentional. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. examples/ios/RunAnywhereAI/IOS_SAMPLE_APP_DOCUMENTATION.md:305
- Draft comment:
Typographical: The bullet point on this line is missing a space after the hyphen. It should be formatted as "- Available frameworks section:" for consistency with the other list items. - Reason this comment was not posted:
Comment was on unchanged code.
4. examples/ios/RunAnywhereAI/IOS_SAMPLE_APP_DOCUMENTATION.md:310
- Draft comment:
Typographical: The bullet point on this line is missing a space after the hyphen. It should be formatted as "- Models list (per framework):" for consistency with the other list items. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. examples/ios/RunAnywhereAI/RunAnywhereAI/App/RunAnywhereAIApp.swift:323
- Draft comment:
Typographical/lexicographical comment: The log message on this line reads "✅ LlamaCPP Core registered" but the adapter is named LlamaCPPCoreAdapter. Consider removing the space in the log message (i.e. "LlamaCPPCore registered") for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a very minor stylistic comment about spacing in a log message. The comment argues for consistency between the adapter class name "LlamaCPPCoreAdapter" and the log message "LlamaCPP Core registered". However, log messages are meant for human readability, not strict technical consistency. "LlamaCPP Core" with a space is actually more readable than "LlamaCPPCore" without one. The class name follows camelCase conventions where spaces aren't allowed, but log messages don't have that constraint. This falls under "obvious or unimportant" comments. It's also purely stylistic with no functional impact. The rules specifically say "Do NOT make comments that are obvious or unimportant." Could there be a legitimate reason for consistency here? Perhaps the project has strict logging conventions where technical names should match exactly? Maybe there's a logging parser that expects specific formats? Even if there were strict logging conventions, there's no evidence of that in the diff. Looking at other log messages in the file, they use human-readable formats like "WhisperKit registered", "ONNX Runtime registered", etc. - these are readable names, not exact class names. The space makes the log more readable for humans, which is the primary purpose of logging. This comment should be deleted. It's a minor stylistic suggestion about spacing in a log message that doesn't affect functionality. Log messages are meant to be human-readable, and "LlamaCPP Core" with a space is more readable than "LlamaCPPCore" without one. This falls under unimportant comments that shouldn't be made.
6. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:40
- Draft comment:
There appears to be a typographical inconsistency in the doc comments. While the other parameter descriptions are indented with 4 spaces followed by///, this line shows no indentation and an extra slash (i.e.///with an extra slash or missing indentation). Please fix the formatting so that it aligns with the other lines (e.g., 4 spaces then///). - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_q1zh6XZbIM4RENfA
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
73 files reviewed, 8 comments
| @@ -0,0 +1,5 @@ | |||
| module CRunAnywhereCore { | |||
| header "ra_types.h" | |||
| header "ra_onnx_bridge.h" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Missing ra_llamacpp_bridge.h header - this file exists in the include directory and is referenced by LlamaCPPService.swift
| header "ra_onnx_bridge.h" | |
| header "ra_onnx_bridge.h" | |
| header "ra_llamacpp_bridge.h" |
Should ra_llamacpp_bridge.h be included here since LlamaCPPService.swift imports CRunAnywhereCore and uses LlamaCPP-specific functions?
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/module.modulemap
Line: 3:3
Comment:
**logic:** Missing `ra_llamacpp_bridge.h` header - this file exists in the include directory and is referenced by LlamaCPPService.swift
```suggestion
header "ra_onnx_bridge.h"
header "ra_llamacpp_bridge.h"
```
Should `ra_llamacpp_bridge.h` be included here since LlamaCPPService.swift imports CRunAnywhereCore and uses LlamaCPP-specific functions?
How can I resolve this? If you propose a fix, please make it concise.| #define RA_AUDIO_CONFIG_STT_DEFAULT { 16000, 1, 16, RA_AUDIO_FORMAT_PCM_F32 } | ||
|
|
||
| // Default audio config for TTS (22050Hz mono) | ||
| #define RA_AUDIO_CONFIG_TTS_DEFAULT { 22050, 1, 16, RA_AUDIO_FORMAT_PCM_F32 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Audio config defaults use PCM_F32 format but specify 16 bits per sample in comments - these should be consistent
| #define RA_AUDIO_CONFIG_STT_DEFAULT { 16000, 1, 16, RA_AUDIO_FORMAT_PCM_F32 } | |
| // Default audio config for TTS (22050Hz mono) | |
| #define RA_AUDIO_CONFIG_TTS_DEFAULT { 22050, 1, 16, RA_AUDIO_FORMAT_PCM_F32 } | |
| // Default audio config for STT (16kHz mono) | |
| #define RA_AUDIO_CONFIG_STT_DEFAULT { 16000, 1, 32, RA_AUDIO_FORMAT_PCM_F32 } | |
| // Default audio config for TTS (22050Hz mono) | |
| #define RA_AUDIO_CONFIG_TTS_DEFAULT { 22050, 1, 32, RA_AUDIO_FORMAT_PCM_F32 } |
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_types.h
Line: 75:78
Comment:
**style:** Audio config defaults use PCM_F32 format but specify 16 bits per sample in comments - these should be consistent
```suggestion
// Default audio config for STT (16kHz mono)
#define RA_AUDIO_CONFIG_STT_DEFAULT { 16000, 1, 32, RA_AUDIO_FORMAT_PCM_F32 }
// Default audio config for TTS (22050Hz mono)
#define RA_AUDIO_CONFIG_TTS_DEFAULT { 22050, 1, 32, RA_AUDIO_FORMAT_PCM_F32 }
```
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.| // Core SDK dependency for result types (NativeTTSSynthesisResult, etc.) | ||
| api(project.parent!!.parent!!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: uses parent navigation to reference core SDK dependency which may be fragile in composite builds. Would using a more explicit dependency reference be more maintainable than parent navigation?
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/modules/runanywhere-core-jni/build.gradle.kts
Line: 50:51
Comment:
**style:** uses parent navigation to reference core SDK dependency which may be fragile in composite builds. Would using a more explicit dependency reference be more maintainable than parent navigation?
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.| #if __has_include("types.h") | ||
| #include "ra_types.h" | ||
| #else | ||
| #include "ra_types.h" | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The conditional include logic may fail if ra_types.h is not found in either location, potentially causing compilation errors
| #if __has_include("types.h") | |
| #include "ra_types.h" | |
| #else | |
| #include "ra_types.h" | |
| #endif | |
| #if __has_include("ra_types.h") | |
| #include "ra_types.h" | |
| #elif __has_include("../capabilities/ra_types.h") | |
| #include "../capabilities/ra_types.h" | |
| #else | |
| #error "Cannot find ra_types.h - check include paths" | |
| #endif |
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_llamacpp_bridge.h
Line: 35:39
Comment:
**logic:** The conditional include logic may fail if ra_types.h is not found in either location, potentially causing compilation errors
```suggestion
#if __has_include("ra_types.h")
#include "ra_types.h"
#elif __has_include("../capabilities/ra_types.h")
#include "../capabilities/ra_types.h"
#else
#error "Cannot find ra_types.h - check include paths"
#endif
```
How can I resolve this? If you propose a fix, please make it concise.| actual override fun streamProcess(input: LLMInput): Flow<LLMGenerationChunk> = flow { | ||
| if (!isServiceInitialized) { | ||
| throw IllegalStateException("LlamaCppService not initialized") | ||
| } | ||
|
|
||
| logger.info("streamProcess() called with ${input.messages.size} messages") | ||
|
|
||
| // Extract prompt and system prompt from messages | ||
| val (prompt, systemPrompt) = extractPromptFromMessages(input.messages, input.systemPrompt) | ||
|
|
||
| logger.info("Stream prompt: ${prompt.take(100)}...") | ||
|
|
||
| val options = input.options ?: RunAnywhereGenerationOptions( | ||
| maxTokens = configuration.maxTokens, | ||
| temperature = configuration.temperature.toFloat(), | ||
| streamingEnabled = true | ||
| ) | ||
|
|
||
| var chunkIndex = 0 | ||
| var tokenCount = 0 | ||
| val maxTokens = options.maxTokens | ||
|
|
||
| logger.info("Starting stream with maxTokens=$maxTokens") | ||
|
|
||
| // Stream with C++ backend (chat template applied in C++) | ||
| coreService.generateStream( | ||
| prompt = prompt, | ||
| systemPrompt = systemPrompt, | ||
| maxTokens = maxTokens, | ||
| temperature = options.temperature | ||
| ) { token -> | ||
| val currentChunk = chunkIndex++ | ||
| val currentTokens = tokenCount++ | ||
| val isComplete = currentTokens >= maxTokens | ||
|
|
||
| logger.info("Stream token #$currentTokens: '$token'") | ||
|
|
||
| // Note: We can't emit from inside a callback, so we collect tokens | ||
| // This is a limitation - for true streaming, use generateStream directly | ||
| true // continue | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Streaming implementation doesn't actually emit Flow elements - the callback approach can't work with Flow emissions. Consider using a different approach like Channel or collecting tokens in a list then emitting them. How should the streaming implementation properly emit Flow when the native callback can't directly emit to the Flow?
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt
Line: 179:220
Comment:
**logic:** Streaming implementation doesn't actually emit Flow elements - the callback approach can't work with Flow emissions. Consider using a different approach like Channel or collecting tokens in a list then emitting them. How should the streaming implementation properly emit Flow<LLMGenerationChunk> when the native callback can't directly emit to the Flow?
How can I resolve this? If you propose a fix, please make it concise.| #if __has_include("types.h") | ||
| #include "ra_types.h" | ||
| #else | ||
| #include "ra_types.h" | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Include path fallback logic is redundant - both branches include the same file ra_types.h
| #if __has_include("types.h") | |
| #include "ra_types.h" | |
| #else | |
| #include "ra_types.h" | |
| #endif | |
| #include "ra_types.h" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_onnx_bridge.h
Line: 35:39
Comment:
**logic:** Include path fallback logic is redundant - both branches include the same file `ra_types.h`
```suggestion
#include "ra_types.h"
```
How can I resolve this? If you propose a fix, please make it concise.| close(e) | ||
| } | ||
| close() | ||
| awaitClose { cancel() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: unnecessary call - close() already called in line 325 inside try block, and awaitClose handles cancellation
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt
Line: 326:326
Comment:
**style:** unnecessary call - `close()` already called in line 325 inside try block, and `awaitClose` handles cancellation
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.| let callback: ra_text_stream_callback = { tokenCStr, userData in | ||
| guard let userData = userData else { return false } | ||
| let context = Unmanaged<CallbackContext>.fromOpaque(userData).takeUnretainedValue() | ||
|
|
||
| if context.isCancelled { | ||
| return false | ||
| } | ||
|
|
||
| if let tokenCStr = tokenCStr { | ||
| let token = String(cString: tokenCStr) | ||
| context.continuation.yield(token) | ||
| } | ||
| return true | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The callback implementation looks correct with proper memory management, but there's a potential issue: takeUnretainedValue() is used but the context is passed with passRetained(). This creates a retain/release mismatch that could cause memory issues. Should line 244 use takeRetainedValue() instead of takeUnretainedValue() to match the passRetained() call on line 240?
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift
Line: 242:255
Comment:
**logic:** The callback implementation looks correct with proper memory management, but there's a potential issue: `takeUnretainedValue()` is used but the context is passed with `passRetained()`. This creates a retain/release mismatch that could cause memory issues. Should line 244 use `takeRetainedValue()` instead of `takeUnretainedValue()` to match the `passRetained()` call on line 240?
How can I resolve this? If you propose a fix, please make it concise.…tion and code - Updated documentation to reflect the renaming of LlamaCPPRuntime and its integration with RunAnywhereCore for GGUF models. - Improved thread safety in LlamaCppCoreService's destroy method by implementing a mutex lock. - Adjusted cleanup logic in LlamaCppService to accommodate the new suspend function for resource management. - Added new headers in CRunAnywhereCore module for better organization and access to LlamaCPP functionalities. This commit enhances the clarity and functionality of the SDK by refining the integration of LlamaCPPRuntime and ensuring robust resource management.
|
@greptile please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 9cdf222 in 1 minute and 42 seconds. Click for details.
- Reviewed
159lines of code in7files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt:322
- Draft comment:
In generateFlow, the channel is closed immediately after generateStream returns. Since native streaming currently emits the full response as a single token, this is acceptable—but consider revisiting the structure when true token-by-token streaming becomes available. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt:36
- Draft comment:
During initialization, the service checks if it is already initialized and calls cleanup before re-initializing. Ensure that cleanup properly releases all native resources to avoid any race conditions. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/module.modulemap:2
- Draft comment:
The module map now explicitly includes 'ra_llamacpp_bridge.h' and 'ra_core.h'. Verify that these header paths are correct in all build settings, including when using the XCFramework distribution. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
4. sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_types.h:74
- Draft comment:
The default audio configuration for STT and TTS has been updated to use 32-bit floats. Please confirm that downstream audio processing components support this higher precision format. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm compatibility with downstream components, which violates the rule against asking for confirmation or ensuring behavior is intended. It does not provide a specific code suggestion or ask for a specific test to be written.
5. sdk/runanywhere-swift/Sources/CRunAnywhereCore/include/ra_llamacpp_bridge.h:32
- Draft comment:
The conditional inclusion of 'ra_types.h' has been removed, now always using a direct include. Ensure that 'ra_types.h' is reliably available in all build environments. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure thatra_types.his reliably available in all build environments. This falls under the rule of not asking the author to ensure something is tested or verified. The comment does not provide a specific suggestion or point out a specific issue with the code change.
Workflow ID: wflow_3cnqUZnPdOfhBFpa
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
Tip For best results, initiate chat on the files or code changes. @sanchitmonga22: I've already provided a comprehensive review and triggered the detailed analysis in my previous comment. The review process is complete. Since you're asking again, let me highlight a few critical action items to address before moving this PR out of draft status: 🔴 Critical Items
✅ What Looks Good
📝 Next Steps
Let me know if you need detailed feedback on specific files! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 73ca174 in 1 minute and 41 seconds. Click for details.
- Reviewed
3359lines of code in27files - Skipped
1files when reviewing. - Skipped posting
9draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. TelemetryEventPayload.swift:80
- Draft comment:
Consider adding a protocol method (e.g.toDictionary()) to AnalyticsEventData to replace Mirror-based extraction for better performance and type-safety. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. AnalyticsQueueManager.swift:125
- Draft comment:
The exponential backoff in processBatch uses pow(2, attempt) as delay, which can become high. Verify that maxRetries (3) and delay (2^attempt seconds) align with desired performance. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. AuthenticationService.swift:157
- Draft comment:
Ensure that KeychainManager.shared.store/retrieve methods properly handle errors and potential race conditions. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
4. ensureDeviceRegistered (in ServiceContainer.swift):214
- Draft comment:
Consider reviewing the retry logic and wait loop for device registration; the number of attempts and sleep intervals should balance timely registration with not blocking initialization. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. ServiceContainer.swift:268
- Draft comment:
The lazy asynchronous initialization of services is well-structured. Ensure that potential race conditions in accessing _syncCoordinator and similar properties are fully tested. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%None
6. RunAnywhere.swift:89
- Draft comment:
Initialization logic in RunAnywhere appears comprehensive. Confirm that the keychain storage in production mode is secured and does not expose sensitive data. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%None
7. APIClient.swift:83
- Draft comment:
Review error parsing in the POST method. Consider logging or returning more structured error information instead of concatenated strings. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
8. RemoteTelemetryDataSource.swift:144
- Draft comment:
The conversion using TelemetryEventPayload works well; ensure that the key conversion with camelCaseToSnakeCase does not miss custom mappings. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
9. ServiceContainer.swift:286
- Draft comment:
The bootstrap functions handle sync and async initialization well. Ensure that error handling in device registration (especially in development mode) safely falls back to using a mock device ID. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_fbICJ0QOGZr6KJJn
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 73ca174 in 4 minutes and 22 seconds. Click for details.
- Reviewed
8359lines of code in73files - Skipped
0files when reviewing. - Skipped posting
12draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPCoreAdapter.swift:40
- Draft comment:
Consider making the memory threshold check more configurable. Currently, it uses ProcessInfo.physicalMemory and a fixed 70% multiplier; a more adaptive strategy might help on devices with varying available memory. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is suggesting a code quality improvement - making the 70% threshold configurable instead of hardcoded. However, this feels like a "nice to have" suggestion rather than identifying a clear bug or issue. The comment uses phrases like "Consider" and "might help", which are soft suggestions rather than identifying a definite problem. The rules state that comments should be about clear code changes required, not speculative improvements. The comment doesn't point to a specific bug or issue - it's more of an architectural suggestion about making something more flexible. This seems to fall into the category of speculative comments ("might help on devices with varying available memory") rather than identifying a definite issue. This could be a legitimate code quality suggestion if the hardcoded 70% value is genuinely problematic. Perhaps in production use, this fixed threshold could cause issues on certain devices. The comment might be pointing out a real limitation that should be addressed before merging. While the 70% threshold might not be perfect for all scenarios, the comment doesn't provide evidence that it's actually causing problems. It's a speculative suggestion ("might help") rather than identifying a concrete issue. The rules explicitly state not to make speculative comments. If this were a critical issue, the comment should be more definitive. This comment should be deleted. It's a speculative suggestion using soft language ("Consider", "might help") rather than identifying a clear code change that's required. It doesn't point to a specific bug or problem, just suggests that something could potentially be better in some scenarios.
2. sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPService.swift:228
- Draft comment:
The generateStream implementation properly creates and releases the callback context with Unmanaged.passRetained/release. Consider adding explicit cancellation support (e.g. checking for Task cancellation) to avoid potential runaway streaming if the generation takes too long. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 40% vs. threshold = 50% This is a new file being added, so all the code is new. The comment is about thegenerateStreammethod starting at line 211. The suggestion is to add Task cancellation checking. While there is a manualcancel()method, the comment is pointing out that if the Task itself is cancelled (e.g., the caller cancels the task), the code doesn't check for that. This would be a valid improvement - checkingTask.isCancelledbefore or during the streaming operation. However, the comment uses language like "Consider adding" which is somewhat speculative and not a definitive issue. The code as written will work, it just won't respond to Swift's structured concurrency cancellation. This is a code quality suggestion rather than a bug. The comment is actionable and clear about what to do. The comment is a "consider" suggestion rather than pointing out a definite bug. The code will function correctly as written - it has its own cancellation mechanism via thecancel()method. The lack of Task cancellation checking is a design choice, not necessarily an error. Without seeing how this API is intended to be used, it's unclear if Task cancellation support is actually needed. While it's true the code works as-is, adding Task cancellation support is a best practice for async Swift code and would make the API more composable with Swift's structured concurrency. The comment is actionable and suggests a clear improvement. However, it does use "consider" language which makes it somewhat speculative. This is a reasonable code quality suggestion for improving cancellation handling in async code. However, it's phrased as a "consider" suggestion rather than identifying a clear bug. Given the rules emphasize removing speculative comments and only keeping comments with strong evidence of issues, this falls into a gray area.
3. sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swift:242
- Draft comment:
The downloadFile helper using withCheckedThrowingContinuation is well structured; however, consider adding explicit handling for task cancellation to ensure robustness during network delays. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXSTTService.swift:288
- Draft comment:
The final decode loop in streamTranscribe (using 'while ra_stt_is_ready') might risk busy-waiting. Consider inserting a small delay or yield to avoid CPU spin. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. sdk/runanywhere-swift/Sources/ONNXTTSService.swift:306
- Draft comment:
The conversion from float samples to 16-bit PCM in convertToPCMData uses a simple linear scaling. Verify that potential clipping and rounding issues are adequately handled for all expected sample ranges. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPServiceProvider.swift:85
- Draft comment:
The asynchronous retrieval of the model path from RunAnywhere.availableModels is well-handled. Ensure robust error paths when multiple models exist or model lookup fails. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment starts with "Ensure" which is a red flag according to the rules - comments that ask to "ensure" or "verify" are typically not actionable. Looking at the actual code, the error handling appears complete: async errors are propagated, missing models throw an error with a clear message, and the logging is appropriate. The comment doesn't point to a specific bug or missing error case - it's just asking the author to make sure error handling is robust, which it already appears to be. The mention of "multiple models exist" doesn't make sense in this context since we're looking up by ID which should be unique. Maybe there's a subtle edge case I'm missing? Perhaps the comment is suggesting that if multiple models have the same ID, the code should handle that explicitly? Or maybe there's a concern about what happens ifavailableModels()returns an empty array? Even if there are edge cases, the comment doesn't specify what they are or what concrete action to take. An empty array would result inmodelInfobeing nil, which is already handled. Multiple models with the same ID would be a data integrity issue, not something this code should handle specially. The comment is too vague and doesn't provide actionable guidance. This comment should be deleted. It starts with "Ensure" which violates the rule against asking authors to verify things. The error handling in the code is already comprehensive, and the comment doesn't identify a specific missing error case or provide actionable guidance for improvement.
7. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Extensions/RunAnywhere+Frameworks.swift:72
- Draft comment:
Event publishing is done via fire-and-forget Task blocks. Consider handling potential cancellation or errors in event publication, although this is low-risk if events are non-critical. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. sdk/runanywhere-swift/Sources/RunAnywhere/Public/Models/GenerationOptions.swift:48
- Draft comment:
Default generation options now use a temperature of 0.8, matching LLM.swift defaults. This consistency is good. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, stating that the default generation options now use a temperature of 0.8, matchingLLM.swiftdefaults. It does not provide any actionable feedback or suggestions for improvement.
9. sdk/runanywhere-swift/Sources/ONNXRuntime/ONNXDownloadStrategy.swift:133
- Draft comment:
The recursive file search (findOnnxFileRecursive) could be inefficient in deeply nested directories. Consider documenting the expected directory structures and potential performance impact. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Constants/SDKConstants.swift:64
- Draft comment:
Constants are well organized and the defaults (e.g. temperature, cacheSizeLimit) are clearly defined. No issues noted. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
11. examples/ios/RunAnywhereAI/RunAnywhereAI/App/RunAnywhereAIApp.swift:314
- Draft comment:
The log message on this line reads "✅ LlamaCPP Core registered" while the adapter is namedLlamaCPPCoreAdapter. Consider removing the space in "LlamaCPP Core" (e.g., "LlamaCPPCore registered") for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a very minor stylistic comment about log message formatting. The comment argues for consistency between the class nameLlamaCPPCoreAdapterand the log message "LlamaCPP Core registered". However, log messages are typically human-readable and don't need to match class names exactly. Looking at other examples in the code: "WhisperKit registered" (class: WhisperKitAdapter), "ONNX Runtime registered" (class: ONNXAdapter), "Foundation Models registered" (class: FoundationModelsAdapter) - these all use human-readable names with spaces, not exact class names. The comment is suggesting a change that would actually make the code LESS consistent with the existing pattern. This is clearly not a bug, just a minor style preference that goes against the existing pattern in the codebase. Could the author have intentionally used "LlamaCPP Core" with a space to make it more readable, following the pattern of other log messages in the file? Am I being too quick to dismiss this as purely stylistic when there might be a legitimate consistency concern? Looking at the broader context, all other log messages use human-readable names with spaces (e.g., "ONNX Runtime", "Foundation Models"), not exact class names. The current "LlamaCPP Core" with a space is actually MORE consistent with the existing pattern. The comment is suggesting a change that would make it less consistent. This comment should be deleted. It's a minor stylistic suggestion that actually goes against the existing pattern in the codebase where log messages use human-readable names with spaces rather than exact class names. The current implementation is consistent with other log messages in the file.
12. sdk/runanywhere-swift/Sources/LlamaCPPRuntime/LlamaCPPServiceProvider.swift:63
- Draft comment:
Typo: The string "smollm" might be a misspelling. If it is intended to be "smalllm" (or another variation), please correct it. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% SmolLM is a legitimate model family from Hugging Face. The name is intentionally spelled "Smol" (not "Small") as a stylistic choice. This is similar to how other models have non-standard spellings (like "Qwen" instead of "Queen"). The comment is incorrectly identifying this as a typo when it's actually the correct name of a real model family. This comment should be deleted because it's factually incorrect. I might be wrong about SmolLM being a real model - perhaps I'm confusing it with something else, or maybe it's a very new model that the automated tool wouldn't know about. However, the automated tool shouldn't be making speculative comments about potential typos without strong evidence. Even if I'm uncertain about SmolLM's existence, the comment violates the rules because it's speculative ("might be a misspelling") and asks the author to confirm/correct something without strong evidence. The tool should not make comments about potential typos in string literals that could be legitimate model names. This comment should be deleted. It's a speculative comment suggesting a possible typo without strong evidence. "smollm" is actually the correct name for the SmolLM model family, making this comment factually incorrect.
Workflow ID: wflow_FygADD8pvNrYygpc
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
73 files reviewed, 2 comments
| } catch (e: Exception) { | ||
| close(e) | ||
| } | ||
| awaitClose { cancel() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: awaitClose already handles cancellation, no need to explicitly call cancel() here
| awaitClose { cancel() } | |
| awaitClose { } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt
Line: 331:331
Comment:
**style:** `awaitClose` already handles cancellation, no need to explicitly call `cancel()` here
```suggestion
awaitClose { }
```
How can I resolve this? If you propose a fix, please make it concise.| actual override fun streamProcess(input: LLMInput): Flow<LLMGenerationChunk> = flow { | ||
| if (!isServiceInitialized) { | ||
| throw IllegalStateException("LlamaCppService not initialized") | ||
| } | ||
|
|
||
| logger.info("streamProcess() called with ${input.messages.size} messages") | ||
|
|
||
| // Extract prompt and system prompt from messages | ||
| val (prompt, systemPrompt) = extractPromptFromMessages(input.messages, input.systemPrompt) | ||
|
|
||
| logger.info("Stream prompt: ${prompt.take(100)}...") | ||
|
|
||
| val options = input.options ?: RunAnywhereGenerationOptions( | ||
| maxTokens = configuration.maxTokens, | ||
| temperature = configuration.temperature.toFloat(), | ||
| streamingEnabled = true | ||
| ) | ||
|
|
||
| var chunkIndex = 0 | ||
| var tokenCount = 0 | ||
| val maxTokens = options.maxTokens | ||
|
|
||
| logger.info("Starting stream with maxTokens=$maxTokens") | ||
|
|
||
| // Stream with C++ backend (chat template applied in C++) | ||
| coreService.generateStream( | ||
| prompt = prompt, | ||
| systemPrompt = systemPrompt, | ||
| maxTokens = maxTokens, | ||
| temperature = options.temperature | ||
| ) { token -> | ||
| val currentChunk = chunkIndex++ | ||
| val currentTokens = tokenCount++ | ||
| val isComplete = currentTokens >= maxTokens | ||
|
|
||
| logger.info("Stream token #$currentTokens: '$token'") | ||
|
|
||
| // Note: We can't emit from inside a callback, so we collect tokens | ||
| // This is a limitation - for true streaming, use generateStream directly | ||
| true // continue | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Flow implementation doesn't emit any chunks. Tokens are collected in callback (line 209-218) but never emitted to the Flow. Consider using callbackFlow with trySend or collect tokens in a list then emit them after callback completes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt
Line: 179:220
Comment:
**logic:** Flow implementation doesn't emit any chunks. Tokens are collected in callback (line 209-218) but never emitted to the Flow. Consider using `callbackFlow` with `trySend` or collect tokens in a list then emit them after callback completes.
How can I resolve this? If you propose a fix, please make it concise.…aCppCoreService - Refactored StorageViewModel to improve model filtering and storage size calculation. - Updated LlamaCppCoreService to utilize AtomicLong for thread-safe backend handle management. - Enhanced various methods in LlamaCppCoreService to ensure lock-free reads and improved resource handling. - Adjusted UI components in TextToSpeechScreen for better layout management. These changes enhance the overall performance and reliability of the SDK, ensuring a more efficient handling of models and backend operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 69a2e54 in 2 minutes and 23 seconds. Click for details.
- Reviewed
350lines of code in8files - Skipped
0files when reviewing. - Skipped posting
9draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. examples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/presentation/navigation/AppNavigation.kt:3
- Draft comment:
Removed unused import 'androidx.compose.foundation.background'. Clean and correct removal. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. examples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/presentation/storage/StorageViewModel.kt:200
- Draft comment:
Refactored deletion: extracting 'filteredModels' improves readability and prevents redundant filtering. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. examples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/presentation/tts/TextToSpeechScreen.kt:580
- Draft comment:
Removed a redundant '.then(if (isSystemTTS) Modifier else Modifier)' from the modifier chain. The simplified chain (.width(140.dp).height(50.dp)) is much cleaner. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. sdk/runanywhere-kotlin/modules/runanywhere-core-jni/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/native/bridge/RunAnywhereBridge.kt:108
- Draft comment:
Added @synchronized annotation to 'isBackendLoaded' to ensure thread safety consistent with the loading routines. Good improvement. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining the addition of an annotation for thread safety. It does not suggest any changes, ask for confirmation, or provide a code suggestion. It simply states what was done and why, which violates the rule against purely informative comments.
5. sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt:50
- Draft comment:
Migrated backendHandle to be an AtomicLong with mutex-protected writes, which improves thread safety. Also, the redundant cancel() call in awaitClose has been removed. Overall, these changes enhance safety and clarity in resource management. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it praises the changes made in the code without providing any actionable feedback or suggestions. It does not align with the rules, which prohibit purely informative comments.
6. sdk/runanywhere-swift/ARCHITECTURE.md:60
- Draft comment:
Updated binary target URL and checksum to the new release version. Documentation now matches the latest XCFramework distribution. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. sdk/runanywhere-swift/Binaries/README.md:12
- Draft comment:
Binary distribution instructions updated with the new XCFramework URL and checksum. This ensures users download the correct binary version. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
8. sdk/runanywhere-swift/README.md:60
- Draft comment:
README updated for new version details and iOS 17+ compatibility. The instructions and module selections now reflect current SDK requirements. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
9. sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/llamacpp/LlamaCppCoreService.kt:85
- Draft comment:
Typographical note: The error message in line 85 uses 'LlamaCPP' (with uppercase) while the backend is referenced as 'llamacpp' (lowercase) elsewhere. Consider using consistent casing for clarity. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_0FVgqDRyAzpua2aR
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
…ities
This commit significantly enhances the SDK's text generation capabilities, providing a robust alternative for LLM inference.
Description
Brief description of the changes made.
Type of Change
Testing
Labels
Please add the appropriate label(s):
iOS SDK- Changes to iOS/Swift SDKAndroid SDK- Changes to Android/Kotlin SDKiOS Sample- Changes to iOS example appAndroid Sample- Changes to Android example appChecklist
Screenshots - Attach all the relevant UI changes screenshots for iOS/Android and MacOS/Tablet/large screen sizes
Important
Introduce LlamaCPP backend for text generation in RunAnywhere SDK, replacing LLM.swift, with support for GGUF models and Metal acceleration.
LlamaCPPServicefor text generation using GGUF models with Metal acceleration.LlamaCPPCoreAdapterto integrate LlamaCPP with RunAnywhere SDK.LLMSwiftwithLlamaCPPRuntimeinPackage.swift.RunAnywhere+Frameworks.swiftto registerLlamaCPPAdapter.LlamaCPPServiceProviderfor model handling and registration.GenerationOptions.swiftto includepreferredFramework.SDKConstants.swift.LLMSwiftrelated files and dependencies.This description was created by
for 69a2e54. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Refactor
Chores
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.
Greptile Overview
Greptile Summary
This PR successfully migrates the RunAnywhere SDK from LLM.swift to a unified C++ backend (runanywhere-core) with LlamaCPP for text generation. The migration provides a consistent implementation across iOS and Android platforms.
Key Changes:
LLM.swiftdependency withRunAnywhereCoreBinaryXCFramework containing the unified C++ backendCRunAnywhereCoremodule exposing C API bridge (ra_llamacpp_bridge.h) for cross-platform compatibilityLlamaCPPServiceandLlamaCPPCoreAdapterfor iOS providing native llama.cpp inference with Metal accelerationllama_chat_apply_templatefor all model typesArchitecture Improvements:
passRetained/takeUnretainedValuefor callbacks in Swift and mutex-protected handles in KotlinIssues Found:
streamProcessFlow implementation doesn't emit chunks - tokens collected in callback are never yieldedcancel()call inawaitCloseblockConfidence Score: 4/5
Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant App as iOS/Android App participant SDK as RunAnywhere SDK participant Adapter as LlamaCPP Adapter participant Service as LlamaCPP Service participant Bridge as C Bridge API participant Core as runanywhere-core<br/>(C++ Backend) participant LlamaCPP as llama.cpp Note over App,LlamaCPP: Registration & Initialization App->>SDK: registerFramework(LlamaCPPCoreAdapter) SDK->>Adapter: onRegistration() Adapter->>SDK: ModuleRegistry.registerLLM(LlamaCPPServiceProvider) Note over App,LlamaCPP: Model Loading App->>SDK: loadModel(modelInfo) SDK->>Adapter: loadModel(modelInfo, .textToText) Adapter->>Service: new LlamaCPPService() Adapter->>Service: initialize() Service->>Bridge: ra_create_backend("llamacpp") Bridge->>Core: Create backend instance Core-->>Bridge: backend_handle Service->>Bridge: ra_initialize(handle) Bridge->>Core: Initialize backend Service->>Bridge: ra_text_load_model(handle, modelPath) Bridge->>Core: Load GGUF model Core->>LlamaCPP: llama_load_model_from_file() LlamaCPP-->>Core: model loaded Core-->>Service: RA_SUCCESS Note over App,LlamaCPP: Text Generation (Streaming) App->>SDK: generate(prompt, options) SDK->>Service: generateStream(prompt, config) Service->>Bridge: ra_text_generate_stream(handle, prompt, callback) Bridge->>Core: Start streaming generation Core->>LlamaCPP: llama_chat_apply_template() Note right of LlamaCPP: Apply model's chat template<br/>(Qwen, LFM2, Llama, etc.) Core->>LlamaCPP: llama_decode() loop loop For each token LlamaCPP->>Core: Generated token Core->>Bridge: Invoke callback Bridge->>Service: callback(token, userData) Service->>App: yield token via AsyncStream end Core-->>Service: RA_SUCCESS Note over App,LlamaCPP: Cleanup App->>SDK: cleanup() SDK->>Service: cleanup() Service->>Bridge: ra_text_unload_model(handle) Bridge->>Core: Unload model Service->>Bridge: ra_destroy(handle) Bridge->>Core: Destroy backend Core->>LlamaCPP: llama_free()