[Kotlin-SDK] [Kotlin-Example] KMP SDK refactor and aligning to iOS #250
Conversation
- Refactored event handling to replace EventBus with EventPublisher for improved event tracking and analytics integration. - Deleted obsolete files and streamlined the SDK structure for better maintainability.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughExtensive refactoring consolidating Android implementations, reorganizing package structures from Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120–180 minutes This PR involves extensive refactoring across multiple dimensions with high heterogeneity:
Areas requiring extra attention:
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)
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 |
- Deleted the runanywhere-llm-mlc module and its associated files to streamline the SDK. - Updated various service and model files to enhance maintainability and performance. - Introduced new components for LLM, STT, TTS, and VAD functionalities, ensuring better integration and support for analytics. - Improved event handling and resource management across the SDK.
…tainability - Streamlined various service and model files, enhancing overall performance and maintainability. - Updated event handling mechanisms to improve integration and analytics support. - Refactored configuration and repository files to align with new architectural standards. - Enhanced network services and data handling for better efficiency and reliability. - Improved documentation and code structure across multiple components.
- Removed obsolete NetworkChecker implementation and refactored related files for improved maintainability. - Introduced new DeviceIdentity and DeviceRegistration services for better device management. - Enhanced event handling by replacing EventBus with EventPublisher across various components. - Streamlined download services and added support for multipart data handling. - Updated network configuration and HTTP client implementations for better performance and reliability.
…nator - Deleted MemoryMonitor, MemoryService, AllocationManager, CacheEviction, and related memory management files to streamline the SDK. - Added SyncCoordinator to manage synchronization operations across services, enhancing data consistency and performance. - Updated ConfigurationLoader documentation for clarity on environment-specific config loading. - Refactored ServiceContainer to integrate SyncCoordinator and improve service management.
- Updated LlamaCppServiceProvider to improve model handling with explicit checks for GGUF/GGML formats and common LLM patterns. - Introduced structured output generation capabilities in LLM, including new classes for structured output handling, generation hints, and stream accumulation. - Enhanced VAD services with TTS feedback prevention and debug statistics tracking, aligning with iOS implementations. - Added new audio capture management components for both Android and JVM, improving audio processing capabilities. - Refactored SDK error handling to include structured error codes for better consistency across platforms.
…cy and performance - Updated InMemoryTelemetryDao to enhance event deletion methods with consistent locking mechanisms. - Refactored AndroidAudioCaptureManager to improve audio recording flow and error handling. - Streamlined audio capture management for both Android and JVM platforms, ensuring better integration and performance. - Removed obsolete components and improved code structure across various files for maintainability.
…vements - Introduced factory type aliases for STT, LLM, TTS, VAD, and Speaker Diarization services to streamline service creation. - Added a new ServiceRegistration data class to manage service factory registrations with priority and handling capabilities. - Enhanced ModuleRegistry with factory-based registration methods for STT, LLM, TTS, VAD, and Speaker Diarization services, aligning with iOS patterns. - Updated SDKError to include a new ProviderNotFound error type for improved error handling. - Removed the obsolete MockNetworkService class to clean up the codebase and improve maintainability. - Refactored APIEndpoint to ensure consistency with iOS endpoint definitions and added environment-specific endpoint handling. - Enhanced VAD and STT error handling to include new error types for better clarity and debugging.
|
@coderabbitai please do a detailed review |
|
@greptile please do a detailed review |
|
@sanchitmonga22: I'll conduct a detailed review of the changes in this PR, focusing on:
I'll analyze all the changes thoroughly and provide comprehensive feedback. ✅ Actions performedReview triggered.
|
Greptile's behavior is changing!From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
@greptile please do a detailed review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/platform/StoragePlatform.android.kt (2)
42-53: Add error handling for directory creation failures.The
mkdirs()call doesn't check whether directory creation succeeded. If it fails (e.g., due to insufficient permissions or disk space), the function still returns a path to a non-existent directory, which will causeIOExceptionorFileNotFoundExceptionwhen file operations are attempted later.🔎 Proposed fix to add error handling
actual fun getPlatformBaseDirectory(): String { val context = AndroidPlatformContext.applicationContext // Match iOS pattern: app-specific directory for SDK files // iOS: .applicationSupportDirectory // Android: filesDir/runanywhere val baseDir = context.filesDir.resolve("runanywhere") if (!baseDir.exists()) { - baseDir.mkdirs() + if (!baseDir.mkdirs()) { + throw IllegalStateException("Failed to create base directory: ${baseDir.absolutePath}") + } } return baseDir.absolutePath }
55-66: Add error handling for directory creation failures.Same issue as
getPlatformBaseDirectory(): themkdirs()call doesn't verify success, risking later file operation failures.🔎 Proposed fix to add error handling
actual fun getPlatformTempDirectory(): String { val context = AndroidPlatformContext.applicationContext // Match iOS pattern: temporary directory // iOS: .temporaryDirectory // Android: cacheDir/runanywhere-temp val tempDir = context.cacheDir.resolve("runanywhere-temp") if (!tempDir.exists()) { - tempDir.mkdirs() + if (!tempDir.mkdirs()) { + throw IllegalStateException("Failed to create temp directory: ${tempDir.absolutePath}") + } } return tempDir.absolutePath }sdk/runanywhere-kotlin/src/androidUnitTest/kotlin/test/kotlin/com/runanywhere/sdk/RunAnywhereSTTTest.kt (1)
16-134: Move business logic tests tocommonTest/for cross-platform coverage.This test file is in
androidUnitTest/but tests common business logic (initialization, transcription, streaming, cleanup) without any Android-specific APIs. Platform-specific test directories should only test platform-specific implementations.Based on coding guidelines, move this test file to
commonTest/to ensure the business logic is tested across all supported platforms. ReserveandroidUnitTest/for testing Android-specific implementations likeAndroidSTTServiceor Android-specific optimizations.sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/security/SecureStorage.kt (1)
22-47: Caching strategy may cause unexpected behavior with multiple identifiers.The companion object caches only one
AndroidSecureStorageinstance. Whencreate(identifier)is called with a different identifier, the previous cached instance is silently replaced. This could lead to unexpected behavior if the SDK uses multiple storage identifiers concurrently.🔎 Proposed fix: Use a map to cache multiple instances
companion object { - private var cachedStorage: AndroidSecureStorage? = null + private val cachedStorages = mutableMapOf<String, AndroidSecureStorage>() private var context: Context? = null /** * Initialize Android secure storage with application context * This should be called during SDK initialization */ fun initialize(applicationContext: Context) { context = applicationContext.applicationContext } /** * Create secure storage instance for Android */ fun create(identifier: String): AndroidSecureStorage { val appContext = context ?: throw SDKError.SecurityError("AndroidSecureStorage not initialized. Call initialize(context) first.") // Return cached instance if available for the same identifier - cachedStorage?.let { cached -> - if (cached.identifier == identifier) { - return cached - } - } + cachedStorages[identifier]?.let { return it }sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/security/KeychainManager.kt (1)
68-181: Standardize error handling: All methods should throw exceptions, not return null/false.The error handling is inconsistent and violates KMP best practices. Mutating operations (
saveTokens,deleteTokens,clearAll) throw exceptions, while query operations (getTokens,hasStoredTokens) return null/false. This violates the KMP principle that expect/actual declarations must have identical return types across platforms.Since Swift best practice recommends avoiding try? and returning optional when errors should be thrown, and your guidelines mandate iOS as the source of truth, all operations should throw exceptions consistently. Returning
falsefromhasStoredTokens()on encryption failure conflates "no tokens stored" with "operation failed," hiding real security issues. Similarly, returningnullfromgetTokens()masks encryption errors.gradle/libs.versions.toml (1)
19-20: Typo in section comment.The comment reads
"# Jetpack Composeios h"— appears to be a typo. Should likely be"# Jetpack Compose".🔎 Proposed fix
# ============================================================================ -# Jetpack Composeios h +# Jetpack Compose # ============================================================================sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppService.kt (1)
288-329:streamProcesshas a limitation with callback-based streaming.The comment on lines 325-327 correctly notes that tokens can't be emitted from inside the callback. The current implementation calls the callback but doesn't actually emit tokens to the Flow. This means
streamProcessreturns an empty Flow.The
streamProcessmethod creates a Flow but thecoreService.generateStreamcallback doesn't emit tokens to the Flow collector. Consider usingcallbackFloworchannelFlowto bridge the callback-based API:Proposed fix using callbackFlow
- actual fun streamProcess(input: LLMInput): Flow<LLMGenerationChunk> = flow { + actual fun streamProcess(input: LLMInput): Flow<LLMGenerationChunk> = callbackFlow { if (!isServiceInitialized) { throw IllegalStateException("LlamaCppService not initialized") } // ... setup code ... 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 + trySend(LLMGenerationChunk( + index = chunkIndex++, + text = token, + isComplete = false + )) + true } + channel.close() + awaitClose() }sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppAdapter.kt (1)
64-86: Add framework compatibility check for consistency.The
canHandlemethod checks format and quantization compatibility but doesn't validate framework compatibility viamodel.compatibleFrameworks. ONNXAdapter explicitly checks this withif (model.compatibleFrameworks.isNotEmpty()) { return model.compatibleFrameworks.contains(InferenceFramework.ONNX) }. Add the same check here for consistency:// Check framework compatibility if (model.compatibleFrameworks.isNotEmpty()) { return model.compatibleFrameworks.contains(InferenceFramework.LLAMA_CPP) }This ensures the method properly validates that the model is compatible with the llama.cpp framework.
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/features/tts/AndroidTTSService.kt (2)
123-150: Race condition in state management.Setting
_isSynthesizing = trueat line 132 without acquiringsynthesisLockcreates a race condition. The nestedsynthesize()call at line 140 will acquire the lock and set the flag again, leading to redundant state updates. If two threads callsynthesizeStreamconcurrently, both can set the flag before either acquires the lock.Consider either:
- Acquiring the lock before setting
_isSynthesizing, or- Removing the flag management from this method entirely and relying on the nested
synthesize()call🔎 Option 1: Acquire lock before setting state
) { - if (!isInitialized) { + synthesisLock.withLock { + if (!isInitialized) { + throw SDKError.ComponentNotReady("TTS service not initialized") + } + + _isSynthesizing = true + try { + logger.debug("Starting streaming synthesis") + + // For streaming, split text into sentences and synthesize each + val sentences = text.split(Regex("[.!?]+")).filter { it.trim().isNotEmpty() } + + for (sentence in sentences) { + val audioData = synthesize(sentence.trim(), options) + if (audioData.isNotEmpty()) { + onChunk(audioData) + } + // Small delay between sentences for natural speech rhythm + delay(100) + } + } finally { + _isSynthesizing = false + } - throw SDKError.ComponentNotReady("TTS service not initialized") } - - _isSynthesizing = true - try { - // existing code - } finally { - _isSynthesizing = false - }Wait, that won't work because synthesize() also tries to acquire the lock. Mutex is not reentrant in Kotlin. So Option 1 would deadlock.
Better solution: Remove state management from this method since synthesize() handles it.
) { if (!isInitialized) { throw SDKError.ComponentNotReady("TTS service not initialized") } - _isSynthesizing = true try { logger.debug("Starting streaming synthesis") // For streaming, split text into sentences and synthesize each val sentences = text.split(Regex("[.!?]+")).filter { it.trim().isNotEmpty() } for (sentence in sentences) { val audioData = synthesize(sentence.trim(), options) if (audioData.isNotEmpty()) { onChunk(audioData) } // Small delay between sentences for natural speech rhythm delay(100) } - } finally { - _isSynthesizing = false }Note: Kotlin's
Mutexis non-reentrant, so nested lock acquisition would deadlock. The cleanest fix is to remove redundant state management.
226-236: Cleanup should coordinate with ongoing synthesis.The
cleanup()method modifies shared state (isInitialized,_isSynthesizing) and nullifiestextToSpeechwithout acquiringsynthesisLock. If cleanup is called whilesynthesize()is running, it can lead to:
- NPE if
textToSpeechis accessed after being nullified- Inconsistent state if flags are modified while the synthesis finally block is executing
Consider acquiring the lock in cleanup to ensure safe coordination:
🔎 Suggested fix
override suspend fun cleanup() { + synthesisLock.withLock { withContext(Dispatchers.Main) { textToSpeech?.stop() textToSpeech?.shutdown() textToSpeech = null isInitialized = false _isSynthesizing = false availableTTSVoices.clear() logger.info("Android TTS service cleaned up") } + } }
🟡 Minor comments (9)
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/models/DeviceInfo.kt-44-50 (1)
44-50: Remove redundant API level check.Both branches of the conditional return identical values (
packageInfo?.versionName), making the API level check and suppression annotation unnecessary. TheversionNameproperty has remained consistent across Android API levels.🔎 Proposed fix
appVersion = - if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.P) { - packageInfo?.versionName - } else { - @Suppress("DEPRECATION") - packageInfo?.versionName - }, + packageInfo?.versionName,sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/utils/PlatformUtils.kt-38-46 (1)
38-46: Log the swallowed exception for debugging.The exception is caught but not logged, which could hide permission issues, security exceptions, or platform bugs during development and debugging.
🔎 Proposed fix to add logging
deviceId = try { Settings.Secure.getString( applicationContext.contentResolver, Settings.Secure.ANDROID_ID, ) } catch (e: Exception) { + // Log for debugging - Android ID retrieval failed + android.util.Log.w("PlatformUtils", "Failed to retrieve Android ID", e) null }sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/features/stt/AndroidAudioCaptureManager.kt-62-68 (1)
62-68: Misleading method name:requestPermission()only checks permission status.The method name implies it will request permission, but it only checks if permission is already granted. While the comment clarifies this, the API surface is confusing. Consider renaming to
checkPermission()or documenting more prominently that the actual request must be handled by the calling Activity.sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/security/SecureStorage.kt-78-83 (1)
78-83: Swallowed exception loses diagnostic information.The exception is caught but not logged, making it difficult to diagnose why secure storage is unsupported. Consider logging the error before returning false.
Based on static analysis hints.
🔎 Proposed fix: Log the exception
fun isSupported(): Boolean = try { context != null } catch (e: Exception) { + SDKLogger("AndroidSecureStorage").error("Secure storage support check failed", e) false }Committable suggestion skipped: line range outside the PR's diff.
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/infrastructure/download/AndroidSimpleDownloader.kt-65-72 (1)
65-72: Fix duplicate logging at 10% intervals.The condition
if (percent % 10 == 0)will log multiple times at each 10% threshold. For example, when progress is between 10.0% and 10.9%,percentwill be 10, and the condition will be true repeatedly. This is similar logic to what's in AndroidDownloadImpl.kt (lines 50-57), which correctly trackslastLoggedPercentto avoid duplicates.🔎 Proposed fix
+var lastLoggedPercent = -1 while (input.read(buffer).also { bytesRead = it } != -1) { output.write(buffer, 0, bytesRead) bytesDownloaded += bytesRead // Report progress every 100ms val currentTime = System.currentTimeMillis() if (currentTime - lastReportTime >= 100) { progressCallback?.invoke(bytesDownloaded, totalBytes) lastReportTime = currentTime // Log every 10% if (totalBytes > 0) { val percent = (bytesDownloaded.toDouble() / totalBytes * 100).toInt() - if (percent % 10 == 0) { + if (percent >= lastLoggedPercent + 10) { logger.debug("Download progress: $percent% ($bytesDownloaded / $totalBytes bytes)") + lastLoggedPercent = percent } } } }sdk/runanywhere-kotlin/src/commonMain/kotlin/com/runanywhere/sdk/core/AudioTypes.kt-28-29 (1)
28-29: Verify fileExtension logic for PCM_16BIT.The
fileExtensionproperty returnsrawValuedirectly, which meansPCM_16BITwill have a file extension of"pcm_16bit". Typically, file extensions don't contain underscores, and both PCM variants should likely use"pcm"as the extension. Please verify this matches the iOS implementation and the intended file naming convention.sdk/runanywhere-kotlin/src/commonMain/kotlin/com/runanywhere/sdk/core/AudioTypes.kt-11-14 (1)
11-14: Fix AudioFormat enum serialization to match iOS behavior.The
@Serializableenum currently serializes by enum name (e.g.,"PCM","WAV"), but the iOS version uses a String-backed enum that serializes to its raw value (e.g.,"pcm","wav"). This creates a mismatch if AudioFormat is serialized to JSON.Use
@SerialNameannotations on each enum case with its rawValue to ensure serialized output matches iOS:@Serializable enum class AudioFormat( val rawValue: String, ) { + @SerialName("pcm") PCM("pcm"), + @SerialName("wav") WAV("wav"), + @SerialName("mp3") MP3("mp3"), + @SerialName("opus") OPUS("opus"), + @SerialName("aac") AAC("aac"), + @SerialName("flac") FLAC("flac"), + @SerialName("ogg") OGG("ogg"), + @SerialName("pcm_16bit") PCM_16BIT("pcm_16bit"), ;This ensures AudioFormat serializes to its rawValue, matching the iOS pattern exactly.
sdk/runanywhere-kotlin/.editorconfig-5-6 (1)
5-6: Contradictory rule: wildcard imports disabled while DAOs use explicit imports.The PR updates
AuthTokenDao.ktandConfigurationDao.ktto use explicit imports, yet this config disables theno-wildcard-importsrule. Consider enabling this rule to enforce the explicit import pattern consistently, or document a clear rationale for the exception.sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/data/repositories/TelemetryRepositoryImpl.kt-24-25 (1)
24-25: Remove the unusednetworkServiceparameter from the constructor.The
networkServiceparameter is not referenced anywhere in the implementation. The JVM variant ofTelemetryRepositoryImpldemonstrates this dependency is unnecessary, and the actual implementation usesremoteTelemetryDataSourcefor telemetry submission. Remove this parameter to simplify the constructor and reduce unnecessary dependency injection.
🧹 Nitpick comments (30)
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/foundation/HostAppInfo.kt (1)
8-27: Consider type-safe error handling with sealed classes.The current implementation catches all exceptions and returns a
HostAppInfowith null values, which silently swallows errors. Consider using a sealed class result type for more explicit error handling, which aligns with the project's coding guidelines.As per coding guidelines, sealed classes should be used for type-safe error handling in Kotlin code.
🔎 Example refactor using sealed class result type
Define a result type in
commonMain:sealed class HostAppInfoResult { data class Success(val info: HostAppInfo) : HostAppInfoResult() data class Error(val exception: Exception) : HostAppInfoResult() }Then refactor the function:
-actual fun getHostAppInfo(): HostAppInfo = +actual fun getHostAppInfoResult(): HostAppInfoResult = try { val context = AndroidPlatformContext.getContext() val packageName = context.packageName val packageManager = context.packageManager val appInfo = context.applicationInfo val packageInfo = packageManager.getPackageInfo(packageName, 0) val appName = packageManager.getApplicationLabel(appInfo).toString() val versionName = packageInfo.versionName - HostAppInfo( + HostAppInfoResult.Success(HostAppInfo( identifier = packageName, name = appName, version = versionName, - ) + )) } catch (e: Exception) { - // Return nulls if unable to get app info - HostAppInfo(null, null, null) + HostAppInfoResult.Error(e) }sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/models/DeviceInfo.kt (1)
28-33: Consider logging the caught exception for debugging.The exception is swallowed and
nullis returned for graceful degradation, which is acceptable. However, logging the exception would improve observability and help diagnose issues during development or troubleshooting.🔎 Proposed enhancement
val packageInfo = try { packageManager.getPackageInfo(context.packageName, 0) } catch (e: Exception) { + // Log for debugging while gracefully degrading + android.util.Log.w("DeviceInfo", "Failed to get package info", e) null }sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/utils/PlatformUtils.kt (1)
60-60: LGTM! Clean expression body conversions.The conversion of single-expression functions to expression bodies is idiomatic Kotlin and improves readability.
Also applies to: 78-78, 80-80
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/storage/AndroidPlatformStorage.kt (1)
15-70: Consider wrapping SharedPreferences operations in withContext(Dispatchers.IO).The suspend functions perform synchronous SharedPreferences operations, which could block the calling coroutine and potentially cause ANR issues if called from the main thread. While
apply()is asynchronous for writes, the function itself doesn't suspend.Recommended approach to make operations truly non-blocking
+import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext + override suspend fun putString( key: String, value: String, -) { +) = withContext(Dispatchers.IO) { sharedPreferences.edit().putString(key, value).apply() } -override suspend fun getString(key: String): String? = sharedPreferences.getString(key, null) +override suspend fun getString(key: String): String? = withContext(Dispatchers.IO) { + sharedPreferences.getString(key, null) +}Apply similar wrapping to all other suspend functions.
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/foundation/device/DeviceInfoService.kt (1)
83-94: Expression body refactoring for getArchitecture is safe and idiomatic.The refactoring mirrors the getChipName pattern and preserves exception handling. However, both methods contain duplicate ABI-extraction logic.
Consider extracting the common ABI-retrieval logic into a private helper method to eliminate duplication:
🔎 Proposed refactor for ABI extraction
+ private fun getPrimaryAbi(): String? = + try { + val abis = Build.SUPPORTED_ABIS + abis.takeIf { it.isNotEmpty() }?.get(0) + } catch (e: Exception) { + null + } + - actual fun getChipName(): String? = - try { - // Get primary ABI (architecture) - val abis = Build.SUPPORTED_ABIS - if (abis.isNotEmpty()) { - abis[0] - } else { - null - } - } catch (e: Exception) { - null - } + actual fun getChipName(): String? = + getPrimaryAbi() - actual fun getArchitecture(): String? = - try { - // Get primary ABI (architecture) - same as chip name for Android - val abis = Build.SUPPORTED_ABIS - if (abis.isNotEmpty()) { - abis[0] - } else { - null - } - } catch (e: Exception) { - null - } + actual fun getArchitecture(): String? = + getPrimaryAbi()sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/features/vad/WebRTCVADService.kt (2)
34-39: Unused variable:ttsThresholdMultiplieris set but never read.The
ttsThresholdMultiplierfield is assigned insetTTSThresholdMultiplier()(line 308) but is never used anywhere in the class. Either integrate it into the threshold adjustment logic or remove it to avoid dead code.Additionally,
recentConfidenceValuesis a non-synchronizedMutableListthat could encounter concurrent modification ifprocessAudioChunkis called from multiple threads. Consider using a thread-safe collection or synchronization.🔎 Proposed fix
- private val recentConfidenceValues = mutableListOf<Float>() + private val recentConfidenceValues = java.util.Collections.synchronizedList(mutableListOf<Float>())Or remove
ttsThresholdMultiplierif it's not needed:- private var ttsThresholdMultiplier: Float = 3.0f
131-135: Consider usingArrayDequefor efficient sliding window.
removeAt(0)on anArrayListis O(n) as it shifts all elements. For a frequently-called method likeprocessAudioChunk, consider usingArrayDequewhich provides O(1) removal from the front.🔎 Proposed fix
- private val recentConfidenceValues = mutableListOf<Float>() + private val recentConfidenceValues = ArrayDeque<Float>(maxRecentValues + 1)And update the tracking logic:
// Track for statistics recentConfidenceValues.add(confidence) if (recentConfidenceValues.size > maxRecentValues) { - recentConfidenceValues.removeAt(0) + recentConfidenceValues.removeFirst() }sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/data/config/ConfigurationLoader.kt (1)
30-38: Swallowed IOException hides asset loading failures.The
IOExceptionat line 35 is caught but not logged, making it difficult to diagnose when configuration files are missing or inaccessible. This is inconsistent with the outer exception handler (line 45) which does log.Consider adding debug logging here for consistency:
🔎 Proposed fix
if (context != null) { try { context.assets .open(fileName) .bufferedReader() .use { it.readText() } } catch (e: IOException) { - // File not found in assets - return empty string + SDKLogger("ConfigurationLoader").debug("Asset file not found: $fileName") "" } } else {sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/features/stt/AndroidAudioCaptureManager.kt (2)
159-176: Consider adding synchronization for thread-safe AudioRecord access.The
audioRecordfield is accessed from the recording coroutine (on Dispatchers.IO) and fromstopRecordingInternal()which may be called from different threads. While the_isRecordingflag provides some coordination, explicit synchronization could prevent edge-case race conditions during concurrent stop/cleanup operations.💡 Optional: Add synchronized block
private fun stopRecordingInternal() { + synchronized(this) { try { audioRecord?.stop() audioRecord?.release() audioRecord = null } catch (e: Exception) { logger.error("Error stopping recording: ${e.message}") } _isRecording.value = false _audioLevel.value = 0.0f logger.info("Recording stopped") + } }
182-198: Extract audio level calculation to commonMain as shared utility.This audio level calculation logic is duplicated between
AndroidAudioCaptureManagerandJvmAudioCaptureManager(see relevant snippet). Since this is pure business logic with no platform-specific dependencies, it should be extracted to a shared utility incommonMain/to eliminate duplication and improve maintainability.As per coding guidelines, business logic should be defined in
commonMain/for KMP projects.💡 Example: Shared utility in commonMain
Create a shared utility in
commonMain/:// commonMain/.../AudioLevelCalculator.kt internal object AudioLevelCalculator { fun calculateNormalizedLevel(samples: ShortArray, count: Int): Float { if (count <= 0) return 0.0f var sum = 0.0 for (i in 0 until count) { val sample = samples[i].toDouble() / 32768.0 sum += sample * sample } val rms = sqrt(sum / count).toFloat() val dbLevel = 20 * log10((rms + 0.0001).toDouble()) return ((dbLevel + 60) / 60).coerceIn(0.0, 1.0).toFloat() } }Then simplify the platform implementations:
private fun updateAudioLevel(buffer: ShortArray, count: Int) { - if (count <= 0) return - // ... calculation logic ... - _audioLevel.value = normalizedLevel + _audioLevel.value = AudioLevelCalculator.calculateNormalizedLevel(buffer, count) }sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/security/SecureStorage.kt (2)
166-174: Consider whether returning false on exception is the intended behavior.Unlike other methods that rethrow exceptions as
SDKError,containsKeyswallows exceptions and returnsfalse. While this may be intentional (treating errors as "key not found"), it could mask underlying storage issues.
187-198: Consider whether returning emptySet() on exception is the intended behavior.Similar to
containsKey,getAllKeysswallows exceptions and returns an empty set instead of rethrowing. This could mask underlying storage issues during diagnostics.sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/infrastructure/device/services/DeviceIdentityAndroid.kt (1)
50-67: Consider more readable UUID formatting.Expanding all 16 byte arguments across separate lines makes the UUID structure harder to parse visually. Consider keeping the format call more compact, or grouping logically (e.g., 4-2-2-2-6 pattern matching UUID structure).
Alternative formatting options
Option 1: Keep compact (original style)
- "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x".format( - bytes[0], - bytes[1], - bytes[2], - bytes[3], - bytes[4], - bytes[5], - bytes[6], - bytes[7], - bytes[8], - bytes[9], - bytes[10], - bytes[11], - bytes[12], - bytes[13], - bytes[14], - bytes[15], - ) + "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x".format( + bytes[0], bytes[1], bytes[2], bytes[3], + bytes[4], bytes[5], bytes[6], bytes[7], + bytes[8], bytes[9], bytes[10], bytes[11], + bytes[12], bytes[13], bytes[14], bytes[15] + )Option 2: Use UUID structure grouping
- "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x".format( - bytes[0], - bytes[1], - bytes[2], - bytes[3], - bytes[4], - bytes[5], - bytes[6], - bytes[7], - bytes[8], - bytes[9], - bytes[10], - bytes[11], - bytes[12], - bytes[13], - bytes[14], - bytes[15], - ) + "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x".format( + // time-low (4 bytes) + bytes[0], bytes[1], bytes[2], bytes[3], + // time-mid (2 bytes) + bytes[4], bytes[5], + // time-high-and-version (2 bytes) + bytes[6], bytes[7], + // clock-seq (2 bytes) + bytes[8], bytes[9], + // node (6 bytes) + bytes[10], bytes[11], bytes[12], bytes[13], bytes[14], bytes[15] + )sdk/runanywhere-kotlin/src/commonMain/kotlin/com/runanywhere/sdk/core/ModuleRegistry.kt (1)
765-786: Provider summary utilities only reflect provider registrations, not factory registrations.The
registeredModulesandproviderSummaryproperties are correctly implemented but only reflect provider-based registrations:
registeredModulesuseshasSTT,hasLLM, etc., which check_xxxProvidersproviderSummarycounts entries in_xxxProviderslistsFactory-only registrations (via
registerXXXFactory()) won't be reflected in these summaries. Consider adding factory counts to provide complete visibility, or clarify the naming to indicate these are provider-specific summaries.sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/platform/Checksum.kt (1)
37-40: Consider usingrequire()for precondition checks.The manual file existence check followed by throwing
IllegalArgumentExceptioncan be simplified using Kotlin'srequire()function for more idiomatic code.🔎 Proposed refactor using require()
- val file = File(filePath) - if (!file.exists()) { - throw IllegalArgumentException("File does not exist: $filePath") - } + val file = File(filePath) + require(file.exists()) { "File does not exist: $filePath" }sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/infrastructure/download/AndroidSimpleDownloader.kt (1)
21-26: Add cancellation support for long-running downloads.The download operation lacks cancellation support, which can be problematic for large files or slow connections. Consider checking for cancellation periodically in the download loop using
ensureActive()or checking the coroutine context.🔎 Suggested enhancement
suspend fun download( url: String, destinationPath: String, progressCallback: ((bytesDownloaded: Long, totalBytes: Long) -> Unit)? = null, ): Long = withContext(Dispatchers.IO) { + // Check for cancellation before starting + ensureActive() logger.info("Starting simple download - url: $url, destination: $destinationPath")And within the download loop (around line 55):
while (input.read(buffer).also { bytesRead = it } != -1) { + ensureActive() // Check for cancellation output.write(buffer, 0, bytesRead)sdk/runanywhere-kotlin/.editorconfig (2)
21-22: Consider reducingmax_line_lengthto a more conventional limit.250 characters is considerably higher than typical limits (100–140). Very long lines can impair readability and code reviews. If there are specific patterns requiring this length, consider refactoring those patterns instead.
7-18: Consider a plan to gradually re-enable naming rules.Disabling multiple naming conventions (
backing-property-naming,property-naming,enum-entry-name-case,class-naming) creates technical debt. Consider tracking these as issues to address incrementally, enabling one rule at a time as violations are fixed.SHARED_TASK_NOTES.md (2)
145-157: Consider adding language specifier to fenced code blocks.The directory tree code blocks should specify a language (e.g.,
textorplaintext) for better rendering and to satisfy linting rules.🔎 Proposed fix
-``` +```text data/ ├── sync/ # Sync services (matches iOS)
161-182: Same suggestion: add language specifier.🔎 Proposed fix
-``` +```text infrastructure/ ├── analytics/ # Analytics services (matches iOS)sdk/runanywhere-kotlin/detekt.yml (1)
154-155: Consider re-enabling naming rules after the refactor stabilizes.Disabling the entire
namingrule set removes enforcement of naming conventions. Consider re-enabling this after the major refactoring is complete to maintain consistency.sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/data/repositories/TelemetryRepositoryImpl.kt (1)
185-199: Helper methods are Android-specific and not in the shared interface.
getEventCount()andgetUnsentEventCount()exist only in the Android implementation and are not defined in theTelemetryRepositoryinterface. The JVM implementation usessentEventIdsmap tracking instead. If these methods need to be available across platforms, consider adding them to the interface; otherwise, document them as Android-specific helpers to clarify the design intent.sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/foundation/ServiceContainer.kt (1)
31-36: Consider logging the caught exception for diagnostics.While catching
UninitializedPropertyAccessExceptionis appropriate for lazy initialization, logging the exception would improve diagnostics without affecting the fallback behavior.🔎 Proposed enhancement
val remoteTelemetryDataSource = try { com.runanywhere.sdk.foundation.ServiceContainer.shared.remoteTelemetryDataSource } catch (e: UninitializedPropertyAccessException) { + logger.debug("RemoteTelemetryDataSource not yet initialized (expected in dev mode)") null // Not available yet or not in production mode }Based on learnings, lazy initialization for service dependencies is preferred to avoid memory pressure.
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/data/database/RunAnywhereDatabase.kt (1)
88-112:DatabaseManagersingleton with lazy initialization follows best practices.The
sharedsingleton with lazy database initialization aligns with the coding guideline to use the Service Container pattern and lazy initialization. However,contextislateinitwhich could throw ifgetDatabase()is called beforeinitialize().Consider adding a guard in
getDatabase()to provide a clearer error message:Suggested improvement
suspend fun getDatabase(): RunAnywhereDatabase { + if (!::context.isInitialized) { + throw IllegalStateException("DatabaseManager not initialized. Call DatabaseManager.initialize(context) first.") + } return database ?: RunAnywhereDatabase.getDatabase(context).also { database = it } }sdk/runanywhere-kotlin/modules/runanywhere-core-llamacpp/src/commonMain/kotlin/com/runanywhere/sdk/llm/llamacpp/LlamaCppServiceProvider.kt (2)
54-103: EnhancedcanHandlelogic is comprehensive but should be documented.The new pattern-matching logic for model identification is thorough:
- Checks GGUF/GGML extensions and references
- Detects llamacpp framework markers
- Uses regex for quantization patterns
- Excludes models with other framework markers (ONNX, CoreML, etc.)
Consider adding a KDoc comment explaining the detection strategy for maintainability.
Suggested documentation
+ /** + * Determines if this provider can handle the given model. + * + * Detection strategy (in order): + * 1. GGUF/GGML file extensions or name references + * 2. Explicit llamacpp framework markers + * 3. GGUF quantization patterns (q2_k, q4_0, q5_1, q8_0, etc.) + * 4. Known LLM model family names (unless marked for other frameworks) + */ override fun canHandle(modelId: String?): Boolean {
80-85: Quantization regex pattern is mostly correct but could miss some variants.The pattern
q[2-8]([_-][kK])?([_-][mMsS0])?handles common cases but may not match:
q4_0correctly matches via[_-][mMsS0](the0is included)f16/f32quantizations are not matchedSince F16/F32 are handled in
LlamaCppAdapter.supportedQuantizations, consider adding them here if they appear in model names.sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/features/tts/AndroidTTSService.kt (4)
40-41: Inconsistent naming convention for state fields.The
isInitializedfield lacks an underscore prefix while_isSynthesizinghas one. Since_isSynthesizingis a backing field for the publicisSynthesizingproperty (line 206-207), consider either:
- Adding an underscore to
isInitializedif you plan to expose it publicly later, or- Removing the underscore from
_isSynthesizingif it's meant to remain privateFor consistency, align the naming pattern across similar state fields.
48-93: Consider adding initialization timeout.The
initialize()method usessuspendCancellableCoroutinewithout a timeout. If the TextToSpeech engine fails to invoke the callback, the coroutine will hang indefinitely. Consider wrapping this inwithTimeoutorwithTimeoutOrNullto handle unresponsive TTS initialization gracefully.🔎 Suggested improvement
override suspend fun initialize() = withTimeout(5000L) { // 5 second timeout suspendCancellableCoroutine { cont -> // ... existing code } }
186-190: State management in stop() should be consistent with synthesis lock.Setting
_isSynthesizing = falseat line 188 without acquiringsynthesisLockcan race with thefinallyblocks insynthesize()and streaming methods. Consider whetherstop()should acquire the lock or rely on the synthesis methods' finally blocks to reset the state.However, since
stop()is meant to interrupt ongoing synthesis andtextToSpeech?.stop()is the actual interrupt mechanism, the current approach may be acceptable if the state flag is advisory rather than critical for correctness.
338-416: Consider adding synthesis timeout.Similar to the initialization method,
synthesizeToByteArraycan hang indefinitely if the TTS engine fails to invoke theonDoneoronErrorcallbacks. Consider wrapping the operation inwithTimeoutto handle unresponsive synthesis gracefully.🔎 Suggested improvement
private suspend fun synthesizeToByteArray( text: String, options: TTSOptions, ): ByteArray = withTimeout(30000L) { // 30 second timeout for synthesis suspendCancellableCoroutine { cont -> // ... existing code } }Otherwise, the implementation is solid: proper file cleanup on all paths, good error handling, and correct use of cancellation.
| // TTS feedback prevention (matching iOS) | ||
| override var isTTSActive: Boolean = false | ||
| private set | ||
| private var baseEnergyThreshold: Float = 0.0f | ||
| private var ttsThresholdMultiplier: Float = 3.0f | ||
|
|
||
| // Debug statistics tracking | ||
| private val recentConfidenceValues = mutableListOf<Float>() | ||
| private val maxRecentValues = 20 |
There was a problem hiding this comment.
Consider thread safety for concurrent VAD processing.
The isTTSActive flag and recentConfidenceValues list are accessed and modified in processAudioChunk (lines 665-680), which may be called from multiple threads during concurrent audio processing. Consider adding:
@Volatileannotation toisTTSActive- Synchronization mechanism for
recentConfidenceValues(e.g.,Collections.synchronizedList()or a mutex)
🔎 Proposed fix for thread safety
- override var isTTSActive: Boolean = false
+ @Volatile
+ override var isTTSActive: Boolean = false
private set
private var baseEnergyThreshold: Float = 0.0f
private var ttsThresholdMultiplier: Float = 3.0f
// Debug statistics tracking
- private val recentConfidenceValues = mutableListOf<Float>()
+ private val recentConfidenceValues = Collections.synchronizedList(mutableListOf<Float>())
private val maxRecentValues = 20Based on learnings, this aligns with proper concurrency handling for service components.
🤖 Prompt for AI Agents
In
sdk/runanywhere-kotlin/modules/runanywhere-core-onnx/src/jvmAndroidMain/kotlin/com/runanywhere/sdk/core/onnx/ONNXServiceProviderImpl.kt
around lines 635-643, the isTTSActive flag and recentConfidenceValues list are
not safe for concurrent access from processAudioChunk; mark isTTSActive as
@Volatile and replace recentConfidenceValues with a thread-safe structure (e.g.,
Collections.synchronizedList(mutableListOf<Float>()) or use a dedicated mutex)
and ensure all reads/writes/iterations over recentConfidenceValues are performed
while holding that synchronization (or via synchronizedList methods) to avoid
race conditions.
| val context = | ||
| try { | ||
| // Access Android context if available | ||
| val contextField = ServiceContainer::class.java.getDeclaredField("androidContext") | ||
| contextField.isAccessible = true | ||
| contextField.get(ServiceContainer.shared) as? Context | ||
| } catch (e: Exception) { | ||
| null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find sdk/runanywhere-kotlin -name "ServiceContainer.kt" -o -name "ConfigurationLoader.kt" | head -20Repository: RunanywhereAI/runanywhere-sdks
Length of output: 753
🏁 Script executed:
cat -n sdk/runanywhere-kotlin/src/commonMain/kotlin/com/runanywhere/sdk/foundation/ServiceContainer.kt | head -80Repository: RunanywhereAI/runanywhere-sdks
Length of output: 4778
🏁 Script executed:
cat -n sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/data/config/ConfigurationLoader.kt | head -50Repository: RunanywhereAI/runanywhere-sdks
Length of output: 2234
🏁 Script executed:
cat -n sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/foundation/ServiceContainer.kt | head -50Repository: RunanywhereAI/runanywhere-sdks
Length of output: 2212
🏁 Script executed:
wc -l sdk/runanywhere-kotlin/src/commonMain/kotlin/com/runanywhere/sdk/foundation/ServiceContainer.kt sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/foundation/ServiceContainer.ktRepository: RunanywhereAI/runanywhere-sdks
Length of output: 290
🏁 Script executed:
cat -n sdk/runanywhere-kotlin/src/commonMain/kotlin/com/runanywhere/sdk/foundation/ServiceContainer.kt | tail -100Repository: RunanywhereAI/runanywhere-sdks
Length of output: 4197
🏁 Script executed:
cat -n sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/foundation/ServiceContainer.ktRepository: RunanywhereAI/runanywhere-sdks
Length of output: 2212
Replace reflection-based context access with a proper public API method.
Per KMP guidelines, shared logic should depend on abstractions, not on concrete platform details. Using reflection to access a private androidContext field violates the Service Container pattern by bypassing its encapsulation. Reflection involves additional runtime checks and lookups and is generally recommended to be used sparingly and only when necessary.
Fix by either:
- Adding a public method on ServiceContainer to retrieve the Android context, or
- Injecting the context as a parameter to the
loadResourceFilefunction during initialization.
Additionally, exceptions from reflection operations should be handled gracefully with try-catch blocks rather than being swallowed. Add logging when context resolution fails at line 25:
try {
// Access Android context if available
val contextField = ServiceContainer::class.java.getDeclaredField("androidContext")
contextField.isAccessible = true
contextField.get(ServiceContainer.shared) as? Context
} catch (e: Exception) {
SDKLogger("ConfigurationLoader").debug("Android context not available: ${e.message}")
null
}🧰 Tools
🪛 detekt (1.23.8)
[warning] 25-25: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/data/config/ConfigurationLoader.kt
around lines 19 to 27, the code uses reflection to read a private androidContext
field from ServiceContainer and swallows any exception; replace that reflection
with a proper public API or dependency injection and add logging on failures:
either (A) add a public accessor on ServiceContainer (e.g., getAndroidContext()
or androidContext property) and call that here, or (B) change loadResourceFile
to accept an Android Context parameter provided at initialization, then remove
the reflection block entirely; if you must keep a fallback, catch exceptions and
log the failure via SDKLogger("ConfigurationLoader").debug(...) before returning
null rather than silently swallowing them.
| private fun getApplicationContext(): Context? { | ||
| // Try to get context through reflection (common pattern in KMP) | ||
| return try { | ||
| val activityThread = Class.forName("android.app.ActivityThread") | ||
| val currentApplication = activityThread.getMethod("currentApplication") | ||
| currentApplication.invoke(null) as? Context | ||
| } catch (e: Exception) { | ||
| null | ||
| } | ||
| } |
There was a problem hiding this comment.
Replace reflection-based context retrieval with proper dependency injection.
Using reflection to obtain the application context via ActivityThread.currentApplication() is fragile and unreliable. This approach can fail due to:
- Android version changes or internal API modifications
- ProGuard/R8 obfuscation
- Security manager restrictions
- Non-standard Android environments
When getApplicationContext() returns null, hasPermission() always returns false, potentially blocking legitimate recording attempts.
As per coding guidelines, use the Service Container pattern for dependency injection. Inject the Context through the constructor or a proper DI mechanism.
🔎 Recommended: Constructor injection approach
Modify the class to accept Context as a parameter:
-class AndroidAudioCaptureManager : AudioCaptureManager {
+class AndroidAudioCaptureManager(
+ private val context: Context
+) : AudioCaptureManager {
private val logger = SDKLogger("AudioCapture")
// ...
override suspend fun hasPermission(): Boolean {
return try {
- val context = getApplicationContext()
- if (context != null) {
ContextCompat.checkSelfPermission(
context,
Manifest.permission.RECORD_AUDIO,
) == PackageManager.PERMISSION_GRANTED
- } else {
- logger.warning("Cannot check permission - no application context")
- false
- }
} catch (e: Exception) {
logger.error("Error checking permission: ${e.message}")
false
}
}
-
- private fun getApplicationContext(): Context? {
- // ...
- }
}Update the factory function:
-actual fun createAudioCaptureManager(): AudioCaptureManager = AndroidAudioCaptureManager()
+actual fun createAudioCaptureManager(): AudioCaptureManager {
+ // Context should be provided via Service Container or initialization
+ val context = ServiceContainer.get<Context>()
+ return AndroidAudioCaptureManager(context)
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/features/stt/AndroidAudioCaptureManager.kt
around lines 200-209, the current reflection-based getApplicationContext() is
fragile; replace it by accepting a non-null Context via constructor injection
(or the project's Service Container/DI), remove the reflection method, and use
the injected context wherever hasPermission() or other context-dependent calls
run; update the class factory/constructors and all call sites (and any tests) to
pass a valid applicationContext (or applicationContext ?: throw
IllegalArgumentException) so permission checks never silently fail due to null.
| override fun synthesizeStream( | ||
| text: String, | ||
| options: TTSOptions, | ||
| ): Flow<ByteArray> = | ||
| flow { | ||
| if (!isInitialized) { | ||
| throw SDKError.ComponentNotReady("TTS service not initialized") | ||
| } | ||
|
|
||
| _isSynthesizing = true | ||
| try { | ||
| logger.debug("Starting Flow-based streaming synthesis") | ||
| _isSynthesizing = true | ||
| try { | ||
| logger.debug("Starting Flow-based streaming synthesis") | ||
|
|
||
| // Split text into sentences and synthesize each | ||
| val sentences = text.split(Regex("[.!?]+")).filter { it.trim().isNotEmpty() } | ||
| // Split text into sentences and synthesize each | ||
| val sentences = text.split(Regex("[.!?]+")).filter { it.trim().isNotEmpty() } | ||
|
|
||
| for (sentence in sentences) { | ||
| val audioData = synthesize(sentence.trim(), options) | ||
| if (audioData.isNotEmpty()) { | ||
| emit(audioData) | ||
| for (sentence in sentences) { | ||
| val audioData = synthesize(sentence.trim(), options) | ||
| if (audioData.isNotEmpty()) { | ||
| emit(audioData) | ||
| } | ||
| delay(100) | ||
| } | ||
| delay(100) | ||
| } finally { | ||
| _isSynthesizing = false | ||
| } | ||
| } finally { | ||
| _isSynthesizing = false | ||
| } |
There was a problem hiding this comment.
Same race condition as callback-based streaming.
This method has the identical concurrency issue as the callback-based synthesizeStream (lines 123-150): _isSynthesizing is set at line 164 without acquiring the lock, then synthesize() is called at line 172, which acquires the lock and sets the flag again.
Apply the same fix: remove the redundant state management and rely on the nested synthesize() call to handle the flag correctly.
🔎 Suggested fix
flow {
if (!isInitialized) {
throw SDKError.ComponentNotReady("TTS service not initialized")
}
- _isSynthesizing = true
try {
logger.debug("Starting Flow-based streaming synthesis")
// Split text into sentences and synthesize each
val sentences = text.split(Regex("[.!?]+")).filter { it.trim().isNotEmpty() }
for (sentence in sentences) {
val audioData = synthesize(sentence.trim(), options)
if (audioData.isNotEmpty()) {
emit(audioData)
}
delay(100)
}
- } finally {
- _isSynthesizing = false
}
}📝 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.
| override fun synthesizeStream( | |
| text: String, | |
| options: TTSOptions, | |
| ): Flow<ByteArray> = | |
| flow { | |
| if (!isInitialized) { | |
| throw SDKError.ComponentNotReady("TTS service not initialized") | |
| } | |
| _isSynthesizing = true | |
| try { | |
| logger.debug("Starting Flow-based streaming synthesis") | |
| _isSynthesizing = true | |
| try { | |
| logger.debug("Starting Flow-based streaming synthesis") | |
| // Split text into sentences and synthesize each | |
| val sentences = text.split(Regex("[.!?]+")).filter { it.trim().isNotEmpty() } | |
| // Split text into sentences and synthesize each | |
| val sentences = text.split(Regex("[.!?]+")).filter { it.trim().isNotEmpty() } | |
| for (sentence in sentences) { | |
| val audioData = synthesize(sentence.trim(), options) | |
| if (audioData.isNotEmpty()) { | |
| emit(audioData) | |
| for (sentence in sentences) { | |
| val audioData = synthesize(sentence.trim(), options) | |
| if (audioData.isNotEmpty()) { | |
| emit(audioData) | |
| } | |
| delay(100) | |
| } | |
| delay(100) | |
| } finally { | |
| _isSynthesizing = false | |
| } | |
| } finally { | |
| _isSynthesizing = false | |
| } | |
| override fun synthesizeStream( | |
| text: String, | |
| options: TTSOptions, | |
| ): Flow<ByteArray> = | |
| flow { | |
| if (!isInitialized) { | |
| throw SDKError.ComponentNotReady("TTS service not initialized") | |
| } | |
| try { | |
| logger.debug("Starting Flow-based streaming synthesis") | |
| // Split text into sentences and synthesize each | |
| val sentences = text.split(Regex("[.!?]+")).filter { it.trim().isNotEmpty() } | |
| for (sentence in sentences) { | |
| val audioData = synthesize(sentence.trim(), options) | |
| if (audioData.isNotEmpty()) { | |
| emit(audioData) | |
| } | |
| delay(100) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/features/tts/AndroidTTSService.kt
around lines 155 to 181, remove the redundant local state management that causes
a race: delete the assignment of `_isSynthesizing = true` before calling
`synthesize()` and remove the corresponding `finally` block that sets
`_isSynthesizing = false`; rely on the existing `synthesize()` implementation
(which acquires the lock and manages `_isSynthesizing`) to set and clear the
flag so the flow-based stream no longer races with the nested synthesize call.
| override suspend fun synthesize( | ||
| text: String, | ||
| options: TTSOptions, | ||
| ): ByteArray { | ||
| if (!service.isSynthesizing) { | ||
| service.initialize() | ||
| } | ||
| return service.synthesize(text = text, options = options) | ||
| } |
There was a problem hiding this comment.
Initialization logic is incorrect.
Lines 443-445 check !service.isSynthesizing to decide whether to initialize, but this is wrong:
isSynthesizingindicates whether synthesis is currently running, not whether the service is initialized- If the service is initialized but idle, this will call
initialize()again unnecessarily - Each call to
initialize()creates a newTextToSpeechinstance without cleaning up the old one, potentially causing resource leaks
The service's isInitialized property is private, so the provider cannot check it directly. Consider either:
- Exposing
isInitializedas a public property inAndroidTTSService, or - Tracking initialization state separately in the provider, or
- Making initialization idempotent by checking
textToSpeech != nullin theinitialize()method
🔎 Suggested fix (Option 1)
In AndroidTTSService, expose the initialization state:
override val isSynthesizing: Boolean
get() = _isSynthesizing
+
+ val isInitialized: Boolean
+ get() = this.isInitializedThen in the provider:
override suspend fun synthesize(
text: String,
options: TTSOptions,
): ByteArray {
- if (!service.isSynthesizing) {
+ if (!service.isInitialized) {
service.initialize()
}
return service.synthesize(text = text, options = options)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
sdk/runanywhere-kotlin/src/androidMain/kotlin/com/runanywhere/sdk/features/tts/AndroidTTSService.kt
around lines 439-447, the provider currently checks service.isSynthesizing to
decide initialization which is incorrect and can recreate TextToSpeech instances
causing leaks; make initialization idempotent by modifying
AndroidTTSService.initialize() to return immediately if its internal
textToSpeech instance is non-null (or expose a public isInitialized boolean and
use that in the provider), then update the provider call to simply call
service.initialize() unconditionally (or check the new public isInitialized) so
initialize() won’t create a new TextToSpeech if one already exists.
| } finally { | ||
| urlConnection.disconnect() | ||
| } |
There was a problem hiding this comment.
Add cleanup for temporary file on download failure.
The temporary file created on line 46 ($destinationPath.tmp) is not deleted if an exception occurs during download. This can lead to accumulation of incomplete downloads on disk.
🔎 Suggested enhancement
+val tempPath = "$destinationPath.tmp"
+val tempFile = java.io.File(tempPath)
+
try {
urlConnection.connect()
// ... download logic ...
+} catch (e: Exception) {
+ // Clean up temp file on failure
+ tempFile.delete()
+ throw e
} finally {
urlConnection.disconnect()
}Committable suggestion skipped: line range outside the PR's diff.
| /** | ||
| * Audio format enum matching iOS AudioFormat pattern exactly | ||
| * This is the single source of truth for audio formats across STT, TTS, and VAD | ||
| * | ||
| * iOS reference: Core/Types/AudioTypes.swift | ||
| */ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for iOS AudioTypes or AudioFormat implementations
find . -type f -name "*.swift" | grep -i audio | head -20
# Search for AudioTypes.swift in SDK directories
fd "AudioTypes.swift" -e swift
# Search for AudioFormat enum/struct definitions in iOS or Core directories
rg -n "enum AudioFormat|struct AudioFormat" --type swift -A 15Repository: RunanywhereAI/runanywhere-sdks
Length of output: 2340
🏁 Script executed:
#!/bin/bash
# Read full iOS AudioTypes.swift
echo "=== iOS AudioTypes.swift ==="
cat -n ./sdk/runanywhere-swift/Sources/RunAnywhere/Core/Types/AudioTypes.swift
echo ""
echo "=== Kotlin AudioTypes.kt ==="
# Find and read the Kotlin AudioTypes file
fd "AudioTypes.kt" -e kt | head -5 | xargs -I {} cat -n {}Repository: RunanywhereAI/runanywhere-sdks
Length of output: 2338
Correct documentation: Kotlin AudioFormat does not match iOS exactly.
The iOS AudioFormat enum contains only 6 formats (pcm, wav, mp3, opus, aac, flac), while the Kotlin version adds OGG and PCM_16BIT. Update lines 6-9 to accurately reflect that:
- OGG and PCM_16BIT are Android-only extensions
- The base formats are shared with iOS
- Only platform-specific formats should be added outside the iOS parity set
Per KMP guidelines, copy iOS logic exactly; extensions must be explicitly marked as platform-specific, not claimed as iOS-compatible.
🤖 Prompt for AI Agents
In
sdk/runanywhere-kotlin/src/commonMain/kotlin/com/runanywhere/sdk/core/AudioTypes.kt
around lines 5 to 10, the file header incorrectly states the Kotlin AudioFormat
matches iOS exactly; update the comment to say the Kotlin enum includes
Android-only extensions (OGG and PCM_16BIT) while the base formats (pcm, wav,
mp3, opus, aac, flac) are shared with iOS, and explicitly mark any additional
entries as platform-specific so iOS parity is preserved and platform extensions
are clearly documented.
| /** | ||
| * Get AudioFormat from raw value string | ||
| */ | ||
| fun fromRawValue(value: String): AudioFormat? = entries.find { it.rawValue == value.lowercase() } |
There was a problem hiding this comment.
Fix the case-insensitive comparison logic.
The comparison compares the lowercased input with it.rawValue directly, which is not lowercased. This breaks case-insensitive matching.
🔎 Proposed fix
- fun fromRawValue(value: String): AudioFormat? = entries.find { it.rawValue == value.lowercase() }
+ fun fromRawValue(value: String): AudioFormat? = entries.find { it.rawValue.lowercase() == value.lowercase() }📝 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 fromRawValue(value: String): AudioFormat? = entries.find { it.rawValue == value.lowercase() } | |
| fun fromRawValue(value: String): AudioFormat? = entries.find { it.rawValue.lowercase() == value.lowercase() } |
🤖 Prompt for AI Agents
In
sdk/runanywhere-kotlin/src/commonMain/kotlin/com/runanywhere/sdk/core/AudioTypes.kt
around line 35, the fromRawValue function lowercases only the input but compares
it to it.rawValue unchanged, breaking case-insensitive matching; update the
comparison to perform a case-insensitive match by either lowercasing both sides
(compare value.lowercase() to it.rawValue.lowercase()) or use a case-insensitive
comparison API (value.equals(it.rawValue, ignoreCase = true)) so entries are
found regardless of input case.
| private data class PrioritizedProvider<T>( | ||
| val provider: T, | ||
| val priority: Int, | ||
| val registrationTime: Long = currentTimeMillis(), | ||
| ) | ||
|
|
||
| // Provider lists - protected by synchronized blocks for thread safety | ||
| // Matches iOS @MainActor pattern but using Kotlin's synchronized for cross-platform support | ||
| private val _sttProviders = mutableListOf<STTServiceProvider>() | ||
| private val _vadProviders = mutableListOf<VADServiceProvider>() | ||
| private val _llmProviders = mutableListOf<LLMServiceProvider>() | ||
| private val _ttsProviders = mutableListOf<TTSServiceProvider>() | ||
| private val _vlmProviders = mutableListOf<VLMServiceProvider>() | ||
| private val _wakeWordProviders = mutableListOf<WakeWordServiceProvider>() | ||
| private val _speakerDiarizationProviders = mutableListOf<SpeakerDiarizationServiceProvider>() | ||
| // Now using PrioritizedProvider for priority-based selection | ||
| private val _sttProviders = mutableListOf<PrioritizedProvider<STTServiceProvider>>() | ||
| private val _vadProviders = mutableListOf<PrioritizedProvider<VADServiceProvider>>() | ||
| private val _llmProviders = mutableListOf<PrioritizedProvider<LLMServiceProvider>>() | ||
| private val _ttsProviders = mutableListOf<PrioritizedProvider<TTSServiceProvider>>() | ||
| private val _speakerDiarizationProviders = mutableListOf<PrioritizedProvider<SpeakerDiarizationServiceProvider>>() | ||
|
|
||
| // MARK: - Factory-Based Registrations (iOS Pattern - ServiceRegistry.swift lines 77-90) | ||
|
|
||
| /** | ||
| * Factory-based service registrations matching iOS ServiceRegistry pattern | ||
| * These allow for closure-based service creation with direct createXXX methods | ||
| */ | ||
| private val _sttRegistrations = mutableListOf<ServiceRegistration<STTServiceFactory>>() | ||
| private val _llmRegistrations = mutableListOf<ServiceRegistration<LLMServiceFactory>>() | ||
| private val _ttsRegistrations = mutableListOf<ServiceRegistration<TTSServiceFactory>>() | ||
| private val _vadRegistrations = mutableListOf<ServiceRegistration<VADServiceFactory>>() | ||
| private val _speakerDiarizationRegistrations = mutableListOf<ServiceRegistration<SpeakerDiarizationServiceFactory>>() |
There was a problem hiding this comment.
Critical architectural issue: Dual independent registration systems create confusion and runtime errors.
The introduction of factory-based registrations (_sttRegistrations, _llmRegistrations, etc.) alongside the existing provider-based registrations (_sttProviders, _vadProviders, etc.) creates two independent, non-interoperating systems:
createSTT()only searches_sttRegistrations(factory-based)sttProvider()only searches_sttProviders(provider-based)
This means:
- If a user registers via
registerSTT(provider), callingcreateSTT()will throwProviderNotFound - If a user registers via
registerSTTFactory(), callingsttProvider()will returnnull
This creates significant confusion about which registration pattern to use and can cause unexpected runtime failures.
Recommendation: Bridge the two systems by either:
- Making
createXXX()methods also check provider registrations and callcreateXXXService()on providers - Making
xxxProvider()methods also check factory registrations and create wrapper providers - Deprecating one pattern in favor of the other with clear migration guidance
🔎 Example bridge implementation for createSTT
suspend fun createSTT(
modelId: String? = null,
config: STTConfiguration,
): STTService {
- val registration = synchronized(_sttRegistrations) {
- _sttRegistrations.firstOrNull { it.canHandle(modelId) }
- } ?: throw SDKError.ProviderNotFound("STT provider for model: ${modelId ?: "default"}")
+ // Try factory registrations first
+ val registration = synchronized(_sttRegistrations) {
+ _sttRegistrations.firstOrNull { it.canHandle(modelId) }
+ }
+
+ if (registration != null) {
+ logger.info("Creating STT service: ${registration.name} for model: ${modelId ?: "default"}")
+ return registration.factory(config)
+ }
+
+ // Fall back to provider registrations
+ val provider = sttProvider(modelId)
+ ?: throw SDKError.ProviderNotFound("STT provider for model: ${modelId ?: "default"}")
- logger.info("Creating STT service: ${registration.name} for model: ${modelId ?: "default"}")
- return registration.factory(config)
+ logger.info("Creating STT service: ${provider.name} for model: ${modelId ?: "default"}")
+ return provider.createSTTService(config)
}Committable suggestion skipped: line range outside the PR's diff.
| interface STTServiceProvider { | ||
| suspend fun createSTTService(configuration: STTConfiguration): STTService | ||
|
|
||
| fun canHandle(modelId: String?): Boolean | ||
|
|
||
| val name: String | ||
| val framework: LLMFramework | ||
| val framework: InferenceFramework | ||
| } | ||
|
|
||
| /** | ||
| * Provider for Voice Activity Detection services | ||
| */ | ||
| interface VADServiceProvider { | ||
| suspend fun createVADService(configuration: VADConfiguration): VADService | ||
|
|
||
| fun canHandle(modelId: String): Boolean | ||
|
|
||
| val name: String | ||
| } | ||
|
|
||
| // LLMServiceProvider is now imported from com.runanywhere.sdk.components.llm.LLMServiceProvider | ||
| // LLMServiceProvider is now imported from com.runanywhere.sdk.features.llm.LLMServiceProvider | ||
|
|
||
| /** | ||
| * Provider for Text-to-Speech services | ||
| */ | ||
| interface TTSServiceProvider { | ||
| suspend fun synthesize(text: String, options: com.runanywhere.sdk.components.TTSOptions): ByteArray | ||
| fun synthesizeStream(text: String, options: com.runanywhere.sdk.components.TTSOptions): kotlinx.coroutines.flow.Flow<ByteArray> | ||
| fun canHandle(modelId: String): Boolean = true | ||
| val name: String | ||
| /** Framework this provider supports */ | ||
| val framework: LLMFramework | ||
| } | ||
| suspend fun synthesize( | ||
| text: String, | ||
| options: com.runanywhere.sdk.features.tts.TTSOptions, | ||
| ): ByteArray | ||
|
|
||
| fun synthesizeStream( | ||
| text: String, | ||
| options: com.runanywhere.sdk.features.tts.TTSOptions, | ||
| ): kotlinx.coroutines.flow.Flow<ByteArray> | ||
|
|
||
| /** | ||
| * Provider for Vision Language Model services | ||
| */ | ||
| interface VLMServiceProvider { | ||
| suspend fun analyze(image: ByteArray, prompt: String?): com.runanywhere.sdk.components.VLMOutput | ||
| suspend fun generateFromImage(image: ByteArray, prompt: String, options: com.runanywhere.sdk.generation.GenerationOptions): String | ||
| fun canHandle(modelId: String): Boolean = true | ||
| val name: String | ||
| } | ||
|
|
||
| /** | ||
| * Provider for Wake Word Detection services | ||
| */ | ||
| interface WakeWordServiceProvider { | ||
| suspend fun createWakeWordService(configuration: com.runanywhere.sdk.components.wakeword.WakeWordConfiguration): com.runanywhere.sdk.components.wakeword.WakeWordService | ||
| fun canHandle(modelId: String?): Boolean | ||
| val name: String | ||
|
|
||
| /** Framework this provider supports */ | ||
| val framework: InferenceFramework | ||
| } | ||
|
|
||
| /** | ||
| * Provider for Speaker Diarization services | ||
| */ | ||
| interface SpeakerDiarizationServiceProvider { | ||
| suspend fun createSpeakerDiarizationService(configuration: com.runanywhere.sdk.components.speakerdiarization.SpeakerDiarizationConfiguration): com.runanywhere.sdk.components.speakerdiarization.SpeakerDiarizationService | ||
| suspend fun createSpeakerDiarizationService( | ||
| configuration: com.runanywhere.sdk.features.speakerdiarization.SpeakerDiarizationConfiguration, | ||
| ): com.runanywhere.sdk.features.speakerdiarization.SpeakerDiarizationService | ||
|
|
||
| fun canHandle(modelId: String?): Boolean | ||
|
|
||
| val name: String | ||
| } |
There was a problem hiding this comment.
Major inconsistency: Framework property missing from some provider interfaces.
The provider interfaces have inconsistent framework properties:
Has framework property:
STTServiceProvider(line 805)TTSServiceProvider(line 840)
Missing framework property:
VADServiceProvider(lines 811-817)SpeakerDiarizationServiceProvider(lines 846-854)
This inconsistency makes the API confusing and prevents uniform framework-based queries across all provider types. Given that framework information is important for adapter management and the coding guidelines recommend structured types over strings, all provider interfaces should consistently declare the framework property.
🔎 Recommended fix to add framework property to all providers
interface VADServiceProvider {
suspend fun createVADService(configuration: VADConfiguration): VADService
fun canHandle(modelId: String): Boolean
val name: String
+ val framework: InferenceFramework
} interface SpeakerDiarizationServiceProvider {
suspend fun createSpeakerDiarizationService(
configuration: com.runanywhere.sdk.features.speakerdiarization.SpeakerDiarizationConfiguration,
): com.runanywhere.sdk.features.speakerdiarization.SpeakerDiarizationService
fun canHandle(modelId: String?): Boolean
val name: String
+ val framework: InferenceFramework
}📝 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.
| interface STTServiceProvider { | |
| suspend fun createSTTService(configuration: STTConfiguration): STTService | |
| fun canHandle(modelId: String?): Boolean | |
| val name: String | |
| val framework: LLMFramework | |
| val framework: InferenceFramework | |
| } | |
| /** | |
| * Provider for Voice Activity Detection services | |
| */ | |
| interface VADServiceProvider { | |
| suspend fun createVADService(configuration: VADConfiguration): VADService | |
| fun canHandle(modelId: String): Boolean | |
| val name: String | |
| } | |
| // LLMServiceProvider is now imported from com.runanywhere.sdk.components.llm.LLMServiceProvider | |
| // LLMServiceProvider is now imported from com.runanywhere.sdk.features.llm.LLMServiceProvider | |
| /** | |
| * Provider for Text-to-Speech services | |
| */ | |
| interface TTSServiceProvider { | |
| suspend fun synthesize(text: String, options: com.runanywhere.sdk.components.TTSOptions): ByteArray | |
| fun synthesizeStream(text: String, options: com.runanywhere.sdk.components.TTSOptions): kotlinx.coroutines.flow.Flow<ByteArray> | |
| fun canHandle(modelId: String): Boolean = true | |
| val name: String | |
| /** Framework this provider supports */ | |
| val framework: LLMFramework | |
| } | |
| suspend fun synthesize( | |
| text: String, | |
| options: com.runanywhere.sdk.features.tts.TTSOptions, | |
| ): ByteArray | |
| fun synthesizeStream( | |
| text: String, | |
| options: com.runanywhere.sdk.features.tts.TTSOptions, | |
| ): kotlinx.coroutines.flow.Flow<ByteArray> | |
| /** | |
| * Provider for Vision Language Model services | |
| */ | |
| interface VLMServiceProvider { | |
| suspend fun analyze(image: ByteArray, prompt: String?): com.runanywhere.sdk.components.VLMOutput | |
| suspend fun generateFromImage(image: ByteArray, prompt: String, options: com.runanywhere.sdk.generation.GenerationOptions): String | |
| fun canHandle(modelId: String): Boolean = true | |
| val name: String | |
| } | |
| /** | |
| * Provider for Wake Word Detection services | |
| */ | |
| interface WakeWordServiceProvider { | |
| suspend fun createWakeWordService(configuration: com.runanywhere.sdk.components.wakeword.WakeWordConfiguration): com.runanywhere.sdk.components.wakeword.WakeWordService | |
| fun canHandle(modelId: String?): Boolean | |
| val name: String | |
| /** Framework this provider supports */ | |
| val framework: InferenceFramework | |
| } | |
| /** | |
| * Provider for Speaker Diarization services | |
| */ | |
| interface SpeakerDiarizationServiceProvider { | |
| suspend fun createSpeakerDiarizationService(configuration: com.runanywhere.sdk.components.speakerdiarization.SpeakerDiarizationConfiguration): com.runanywhere.sdk.components.speakerdiarization.SpeakerDiarizationService | |
| suspend fun createSpeakerDiarizationService( | |
| configuration: com.runanywhere.sdk.features.speakerdiarization.SpeakerDiarizationConfiguration, | |
| ): com.runanywhere.sdk.features.speakerdiarization.SpeakerDiarizationService | |
| fun canHandle(modelId: String?): Boolean | |
| val name: String | |
| } | |
| interface STTServiceProvider { | |
| suspend fun createSTTService(configuration: STTConfiguration): STTService | |
| fun canHandle(modelId: String?): Boolean | |
| val name: String | |
| val framework: InferenceFramework | |
| } | |
| /** | |
| * Provider for Voice Activity Detection services | |
| */ | |
| interface VADServiceProvider { | |
| suspend fun createVADService(configuration: VADConfiguration): VADService | |
| fun canHandle(modelId: String): Boolean | |
| val name: String | |
| val framework: InferenceFramework | |
| } | |
| // LLMServiceProvider is now imported from com.runanywhere.sdk.features.llm.LLMServiceProvider | |
| /** | |
| * Provider for Text-to-Speech services | |
| */ | |
| interface TTSServiceProvider { | |
| suspend fun synthesize( | |
| text: String, | |
| options: com.runanywhere.sdk.features.tts.TTSOptions, | |
| ): ByteArray | |
| fun synthesizeStream( | |
| text: String, | |
| options: com.runanywhere.sdk.features.tts.TTSOptions, | |
| ): kotlinx.coroutines.flow.Flow<ByteArray> | |
| fun canHandle(modelId: String): Boolean = true | |
| val name: String | |
| /** Framework this provider supports */ | |
| val framework: InferenceFramework | |
| } | |
| /** | |
| * Provider for Speaker Diarization services | |
| */ | |
| interface SpeakerDiarizationServiceProvider { | |
| suspend fun createSpeakerDiarizationService( | |
| configuration: com.runanywhere.sdk.features.speakerdiarization.SpeakerDiarizationConfiguration, | |
| ): com.runanywhere.sdk.features.speakerdiarization.SpeakerDiarizationService | |
| fun canHandle(modelId: String?): Boolean | |
| val name: String | |
| val framework: InferenceFramework | |
| } |
🤖 Prompt for AI Agents
In
sdk/runanywhere-kotlin/src/commonMain/kotlin/com/runanywhere/sdk/core/ModuleRegistry.kt
around lines 799 to 854, the VADServiceProvider and
SpeakerDiarizationServiceProvider interfaces are missing the framework property
present on STTServiceProvider and TTSServiceProvider; add a val framework:
InferenceFramework to both interfaces, keeping it non-nullable and named exactly
"framework" to match the other providers so consumers can uniformly query
supported frameworks and adapter selection logic remains consistent.
- Updated service registration methods in ModuleRegistry for STT, LLM, TTS, VAD, and Speaker Diarization to enhance readability. - Improved error handling in VADError by adding new error types for better debugging. - Refactored APIEndpoint to ensure consistent environment handling. - Streamlined model discovery logic in ModelDiscoveryService for better maintainability. - Enhanced ErrorContext logging for clearer error reporting.
|
@greptile please review the PR |
- Introduced PlatformAudioProcessor for speaker embedding creation and audio feature extraction. - Added SimpleEnergyVADService for energy-based voice activity detection, replacing the deprecated WebRTCVADService. - Enhanced VADComponent error handling and service registration for improved clarity. - Updated model discovery service to streamline framework inference and model format handling. - Refactored various components for better maintainability and performance across the SDK.
- Introduced .editorconfig for Ktlint to customize rules for Kotlin files, particularly for Compose functions and imports. - Updated build.gradle.kts to include Ktlint plugin configuration for better code quality checks. - Removed outdated detekt-config.yml and adjusted detekt.yml settings to enhance code analysis and maintainability. - Deleted unused model-related files and cleaned up the project structure for improved clarity and performance.
…ntegration - Removed outdated models_info.json file to streamline model management. - Implemented registerModulesAndModels() method in RunAnywhereApplication for explicit model registration, aligning with iOS patterns. - Enhanced SettingsViewModel to utilize SDK's getStorageInfo(), clearCache(), and cleanTempFiles() APIs for better storage management. - Introduced LlamaCPP and ONNX modules for LLM and STT/TTS capabilities, respectively, with detailed model registration and usage instructions. - Added public storage extensions for improved model handling and storage operations, matching iOS functionality.
…y and consistency - Updated RunAnywhereApplication to utilize new artifact type methods for model registration, enhancing readability. - Renamed downloadModel() to startDownload() in ModelSelectionViewModel for better clarity in function purpose. - Improved logging during model download processes to provide clearer feedback on operations. - Streamlined device info collection in Android and JVM implementations to match iOS patterns. - Removed obsolete ModelIntegrityVerifier and related checks to simplify model management.
|
@greptile please do a detailed review |
- Updated SpeechToTextScreen to set the selected model name in the ViewModel for improved user feedback. - Refactored SpeechToTextViewModel to include a method for setting the selected model name, enhancing clarity in model selection. - Improved logging in TextToSpeechViewModel and VoiceAssistantViewModel to use modelId consistently, ensuring better traceability during model load events. - Removed obsolete LlamaCppAdapter and LlamaCppServiceProvider files to streamline the codebase and improve maintainability. - Updated ONNX module to enhance service creation and registration processes, aligning with recent architectural changes.
… management - Deleted SecureStorage.kt and SettingsDataStore.kt files to simplify the data handling architecture. - Updated related components to remove dependencies on these classes, enhancing maintainability and clarity in the codebase.
- Bumped coreVersion to 0.2.6 and commonsVersion to 0.1.2 in build.gradle.kts and gradle.properties files. - Updated checksums in Package.swift for the new versions of XCFrameworks. - Removed outdated unit test files for RunAnywhere STT and AnalyticsNetworkService.
- Introduced new JNI functions to convert Float32 and Int16 PCM audio data to WAV format. - Added a function to retrieve the WAV header size. - Updated CppBridgeTTS to utilize the new audio conversion functions for TTS output. - Enhanced build scripts to ensure proper handling of JNI libraries across modules.
- Added context-specific key for ViewModel to prevent caching issues across different contexts in ModelSelectionBottomSheet. - Enhanced ModelSelectionViewModel to sync currently loaded models with the UI state, ensuring accurate representation of loaded models. - Updated VoiceAssistantViewModel to initialize audio capture service and refresh model states upon screen appearance, improving responsiveness. - Implemented error handling for model loading failures in the SDK, ensuring robust operation during voice processing. - Enhanced storage metrics retrieval for downloaded models, providing better insights into storage usage.
- Set runanywhere.testLocal to true in gradle.properties for both Android and Kotlin SDKs to facilitate local development. - Added device manager and telemetry manager functionalities in the Kotlin SDK, including device registration and telemetry event handling. - Improved analytics event registration to route events through the telemetry manager for better tracking and reporting. - Updated CppBridge to initialize telemetry with device information and handle HTTP callbacks for telemetry events.
- Deleted the .gitmodules file as the associated submodule is no longer needed. - Updated build.gradle.kts to ensure the buildLocalJniLibs task always cleans old JNI libraries before building fresh ones. - Modified build-local.sh to improve backend selection logic and ensure proper handling of JNI libraries, including support for sherpa-onnx libraries.
- Complete telemetry implementation for LLM, STT, TTS events - Device registration with UPSERT and persistent device ID - JWT authentication flow for production/staging environments - Token refresh mechanism - Proper model ID/name tracking (not file paths) - STT streaming telemetry with is_streaming flag - Device info collection (architecture, chip, memory, GPU family) - Production mode single registration, dev mode UPSERT
* chore: Enable local testing and enhance telemetry integration - Set runanywhere.testLocal to true in gradle.properties for both Android and Kotlin SDKs to facilitate local development. - Added device manager and telemetry manager functionalities in the Kotlin SDK, including device registration and telemetry event handling. - Improved analytics event registration to route events through the telemetry manager for better tracking and reporting. - Updated CppBridge to initialize telemetry with device information and handle HTTP callbacks for telemetry events. * clenaup * feat: Add Kotlin SDK telemetry aligned with Swift SDK - Complete telemetry implementation for LLM, STT, TTS events - Device registration with UPSERT and persistent device ID - JWT authentication flow for production/staging environments - Token refresh mechanism - Proper model ID/name tracking (not file paths) - STT streaming telemetry with is_streaming flag - Device info collection (architecture, chip, memory, GPU family) - Production mode single registration, dev mode UPSERT * updates * resolving comments * async update * fixing the final issue identified - telemetry looks good for dev/prod * minor fixes * fix merge conflict
|
Too many files changed for review. |
This reverts commit 1d24b59.
…ing commit history)
|
Too many files changed for review. |
- Update workflow to inject SUPABASE_URL, SUPABASE_ANON_KEY, BUILD_TOKEN from GitHub secrets - Bump VERSION to 0.1.4 - Fixes telemetry not working due to placeholder credentials in 0.1.3
- Remove indentation from heredocs - Use printf instead of echo to avoid newline issues - Strip newlines/carriage returns from secret values - Fixes build failure due to broken string literals
|
Too many files changed for review. |
- commonsVersion: 0.1.3 → 0.1.4 (with Supabase telemetry) - coreVersion: 0.1.3 → 0.1.4 (with llama.cpp b7658) - Updated RACommons checksum for iOS Note: Backend checksums will be updated once core-v0.1.4 release completes
- RABackendLlamaCPP checksum updated for core-v0.1.4 - RABackendONNX checksum updated for core-v0.1.4 - Kotlin SDK version: 0.1.3 → 0.1.4
- runanywhere.coreVersion: 0.1.3 → 0.1.4 - runanywhere.commonsVersion: 0.1.3 → 0.1.4 This file was overriding the defaults in build.gradle.kts
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
Summary by CodeRabbit
Removed Features
Refactored Architecture
✏️ Tip: You can customize this high-level summary in your review settings.
Greptile Summary
This PR refactors event handling by introducing
EventPublisheras a centralized event router that replaces directEventBus.publish()calls throughout the SDK. The new architecture provides intelligent event routing based on destination:Key Changes:
EventPublisher.track()PUBLIC_ONLY→ app developers via EventBus,ANALYTICS_ONLY→ backend telemetry,ALL→ both destinations (default)EventPublisher.track()instead ofEventBus.publish()Architecture Improvements:
Documentation Added:
The refactor maintains backward compatibility with existing EventBus subscriptions while providing the foundation for improved analytics tracking and event management.
Confidence Score: 4/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant Component as SDK Component<br/>(LLM, STT, TTS, VAD) participant EventPublisher as EventPublisher<br/>(Router) participant EventBus as EventBus<br/>(Public) participant Adapter as SDKEventAnalyticsAdapter participant AnalyticsQueue as AnalyticsQueueManager participant TelemetryRepo as TelemetryRepository<br/>(Backend) Note over Component,TelemetryRepo: SDK Initialization Phase Component->>EventPublisher: EventPublisher.initializeWithSDKEventRouting(telemetryRepo) EventPublisher->>EventPublisher: Create AnalyticsEnqueuer lambda EventPublisher-->>Component: Initialized Note over Component,TelemetryRepo: Event Tracking Flow Component->>EventPublisher: EventPublisher.track(LLMEvent.GenerationCompleted) alt destination == PUBLIC_ONLY or ALL EventPublisher->>EventBus: EventBus.publishSDKEvent(event) EventBus->>EventBus: Route to typed publishers<br/>(generationEvents, modelEvents, etc.) EventBus-->>Component: Event published to app end alt destination == ANALYTICS_ONLY or ALL EventPublisher->>Adapter: Wrap SDKEvent as AnalyticsEvent Adapter->>AnalyticsQueue: AnalyticsQueueManager.enqueue(analyticsEvent) AnalyticsQueue->>AnalyticsQueue: Add to queue<br/>(batch when size >= 50) alt Queue threshold reached or flush timer AnalyticsQueue->>TelemetryRepo: Send batch to backend TelemetryRepo-->>AnalyticsQueue: Success/Failure end end Note over Component,TelemetryRepo: EventDestination Controls Routing:<br/>• PUBLIC_ONLY → EventBus only<br/>• ANALYTICS_ONLY → Analytics only<br/>• ALL → Both destinations (default)Context used:
dashboard- CLAUDE.md (source)