[Flutter SDK] Add configuration validation for LLM, STT, and TTS (#450)#456
Open
DevDesai444 wants to merge 13 commits intoRunanywhereAI:mainfrom
Open
[Flutter SDK] Add configuration validation for LLM, STT, and TTS (#450)#456DevDesai444 wants to merge 13 commits intoRunanywhereAI:mainfrom
DevDesai444 wants to merge 13 commits intoRunanywhereAI:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds LLM, STT, and TTS configuration classes with validate() implementations; threads model contextLength through model registry and LLM bridge; and invokes runtime validation in dart_bridge_llm/stt/tts and in public RunAnywhere flows to prevent invalid parameters crossing the FFI boundary. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant RunAnywhere as RunAnywhere API
participant DartBridge as DartBridgeLLM
participant Native as Native/FFI
Client->>RunAnywhere: call generate(prompt, config)
RunAnywhere->>DartBridge: generate(..., modelInfo?.contextLength, params)
DartBridge->>DartBridge: _requireLoadedContextLength()
DartBridge->>DartBridge: _validateGenerationParameters(contextLength,maxTokens,temperature,systemPrompt,streaming)
alt validation passes
DartBridge->>Native: call native generate with validated params
Native-->>DartBridge: generation result / stream
DartBridge-->>RunAnywhere: result / stream
RunAnywhere-->>Client: deliver result/stream
else validation fails
DartBridge-->>RunAnywhere: throw SDKError.validationFailed
RunAnywhere-->>Client: propagate SDKError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+383
to
+396
| void _validateGenerationParameters({ | ||
| required int maxTokens, | ||
| required double temperature, | ||
| String? systemPrompt, | ||
| bool streamingEnabled = false, | ||
| }) { | ||
| LLMConfiguration( | ||
| contextLength: 32768, | ||
| maxTokens: maxTokens, | ||
| temperature: temperature, | ||
| systemPrompt: systemPrompt, | ||
| streamingEnabled: streamingEnabled, | ||
| ).validate(); | ||
| } |
Contributor
There was a problem hiding this comment.
Hardcoded contextLength defeats maxTokens validation
The contextLength is hardcoded to 32768 (the maximum allowed), which means the maxTokens <= contextLength check in LLMConfiguration.validate() will never reject any value under 32768. A user could pass maxTokens: 32768 even though the loaded model may have a much smaller context window (e.g. 2048 or 4096), sending an invalid value across the FFI boundary — exactly what this PR is trying to prevent.
Consider passing the actual model's context length here. Since DartBridgeLLM manages the C++ lifecycle, the real context length may be queryable from the native layer, or you could store it when the model is loaded and pass it through to validation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_llm.dart
Line: 383-396
Comment:
**Hardcoded `contextLength` defeats `maxTokens` validation**
The `contextLength` is hardcoded to `32768` (the maximum allowed), which means the `maxTokens <= contextLength` check in `LLMConfiguration.validate()` will never reject any value under 32768. A user could pass `maxTokens: 32768` even though the loaded model may have a much smaller context window (e.g. 2048 or 4096), sending an invalid value across the FFI boundary — exactly what this PR is trying to prevent.
Consider passing the actual model's context length here. Since `DartBridgeLLM` manages the C++ lifecycle, the real context length may be queryable from the native layer, or you could store it when the model is loaded and pass it through to validation.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+1847
to
+1848
| } on SDKError { | ||
| rethrow; |
Contributor
There was a problem hiding this comment.
Missing parallel SDKError rethrow in generateStream, transcribe, and synthesize
The on SDKError { rethrow; } clause was added here for generate, but the sibling generateStream method (line ~1930) calls DartBridge.llm.generateStream() outside any try-catch. While this works because validation errors propagate naturally (they're thrown synchronously before the stream is returned), it creates an inconsistency in error handling patterns.
More importantly, transcribe (line 684) and synthesize (line 947) both have a generic catch (e) { ... rethrow; } which does preserve the SDKError via rethrow. This means the generate method was the only place actively swallowing SDKError (via throw SDKError.generationFailed('$e')). This fix is correct — just calling it out for completeness that only generate had this masking issue.
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/runanywhere-flutter/packages/runanywhere/lib/public/runanywhere.dart
Line: 1847-1848
Comment:
**Missing parallel `SDKError` rethrow in `generateStream`, `transcribe`, and `synthesize`**
The `on SDKError { rethrow; }` clause was added here for `generate`, but the sibling `generateStream` method (line ~1930) calls `DartBridge.llm.generateStream()` outside any try-catch. While this works because validation errors propagate naturally (they're thrown synchronously before the stream is returned), it creates an inconsistency in error handling patterns.
More importantly, `transcribe` (line 684) and `synthesize` (line 947) both have a generic `catch (e) { ... rethrow; }` which *does* preserve the `SDKError` via `rethrow`. This means the `generate` method was the only place actively swallowing `SDKError` (via `throw SDKError.generationFailed('$e')`). This fix is correct — just calling it out for completeness that only `generate` had this masking issue.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sdk/runanywhere-flutter/packages/runanywhere/lib/features/tts/system_tts_service.dart (1)
15-15: Consider a dedicatedtts_configuration.dart.
TTSConfigurationis now shared by the bridge and the package root export, so keeping it insidesystem_tts_service.dartcouples a reusable model to a concrete service module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/runanywhere-flutter/packages/runanywhere/lib/features/tts/system_tts_service.dart` at line 15, TTSConfiguration is a reusable model but currently defined inside system_tts_service.dart which couples it to the concrete service; extract the class into a new file named tts_configuration.dart, move the TTSConfiguration declaration there, update imports to import 'tts_configuration.dart' wherever TTSConfiguration is referenced (including system_tts_service.dart and the package root / bridge exports), and ensure the package export/entry point exposes TTSConfiguration from the new file instead of from system_tts_service.dart.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_llm.dart`:
- Around line 383-395: The hardcoded context length (32768) in
_validateGenerationParameters causes incorrect validation; update the call to
LLMConfiguration.validate to use the active model's contextLength instead of
32768 by either adding a contextLength parameter to
_validateGenerationParameters (and passing the loaded model.contextLength in the
caller) or performing the maxTokens vs contextLength check in the caller where
model metadata is available; reference the _validateGenerationParameters
function and LLMConfiguration.validate to locate and change the validation input
so oversized maxTokens are rejected before the FFI hop.
---
Nitpick comments:
In
`@sdk/runanywhere-flutter/packages/runanywhere/lib/features/tts/system_tts_service.dart`:
- Line 15: TTSConfiguration is a reusable model but currently defined inside
system_tts_service.dart which couples it to the concrete service; extract the
class into a new file named tts_configuration.dart, move the TTSConfiguration
declaration there, update imports to import 'tts_configuration.dart' wherever
TTSConfiguration is referenced (including system_tts_service.dart and the
package root / bridge exports), and ensure the package export/entry point
exposes TTSConfiguration from the new file instead of from
system_tts_service.dart.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed05361a-d750-489e-86eb-4b58d1448d6e
📒 Files selected for processing (8)
sdk/runanywhere-flutter/packages/runanywhere/lib/features/llm/llm_configuration.dartsdk/runanywhere-flutter/packages/runanywhere/lib/features/stt/stt_configuration.dartsdk/runanywhere-flutter/packages/runanywhere/lib/features/tts/system_tts_service.dartsdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_llm.dartsdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_stt.dartsdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_tts.dartsdk/runanywhere-flutter/packages/runanywhere/lib/public/runanywhere.dartsdk/runanywhere-flutter/packages/runanywhere/lib/runanywhere.dart
sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_llm.dart
Show resolved
Hide resolved
Contributor
Collaborator
|
Noted ✅ |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sdk/runanywhere-flutter/packages/runanywhere/lib/public/runanywhere.dart (1)
768-778: Consider addingSDKErrorrethrow for consistency.
transcribe()at Lines 689-690 explicitly rethrowsSDKErrorbefore the generic catch, buttranscribeWithResult()does not. While the error is still propagated viarethrow, it would be logged as a generic error in telemetry, which may be misleading for validation failures.♻️ Suggested fix for consistency
return STTResult( text: result.text, confidence: result.confidence, durationMs: audioDurationMs, language: result.language, ); + } on SDKError { + rethrow; } catch (e) { // Track transcription failure TelemetryService.shared.trackError(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/runanywhere-flutter/packages/runanywhere/lib/public/runanywhere.dart` around lines 768 - 778, transcribeWithResult() currently logs and rethrows errors only in the generic catch block, causing SDKError instances to be recorded as generic failures; update the try/catch so that SDKError is explicitly caught and rethrown before the generic catch (mirror the pattern used in transcribe()), and ensure the TelemetryService.shared.trackError call in the generic catch uses the original error details when available; look for function transcribeWithResult(), the existing catch block that calls TelemetryService.shared.trackError and logger.error('Transcription failed: $e'), and add a preceding catch for SDKError to rethrow unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@sdk/runanywhere-flutter/packages/runanywhere/lib/public/runanywhere.dart`:
- Around line 768-778: transcribeWithResult() currently logs and rethrows errors
only in the generic catch block, causing SDKError instances to be recorded as
generic failures; update the try/catch so that SDKError is explicitly caught and
rethrown before the generic catch (mirror the pattern used in transcribe()), and
ensure the TelemetryService.shared.trackError call in the generic catch uses the
original error details when available; look for function transcribeWithResult(),
the existing catch block that calls TelemetryService.shared.trackError and
logger.error('Transcription failed: $e'), and add a preceding catch for SDKError
to rethrow unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b321539-7ff5-4955-af39-c2826adfdca0
📒 Files selected for processing (7)
sdk/runanywhere-flutter/packages/runanywhere/lib/features/tts/system_tts_service.dartsdk/runanywhere-flutter/packages/runanywhere/lib/features/tts/tts_configuration.dartsdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_llm.dartsdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_model_registry.dartsdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_tts.dartsdk/runanywhere-flutter/packages/runanywhere/lib/public/runanywhere.dartsdk/runanywhere-flutter/packages/runanywhere/lib/runanywhere.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- sdk/runanywhere-flutter/packages/runanywhere/lib/runanywhere.dart
- sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_tts.dart
- sdk/runanywhere-flutter/packages/runanywhere/lib/features/tts/system_tts_service.dart
Author
|
@Siddhesh2377 can you review this PR ? |
Collaborator
|
Sure @DevDesai444 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #450 by adding the missing Flutter-side configuration validation for LLM, STT, and TTS before invalid values cross the FFI boundary into the native C++ layer.
Flutter already validated VAD, but LLM and STT configuration models were missing and TTS had no
validate()implementation. This PR brings Flutter behavior closer to Swift/Kotlin parity for these components.Problem
Today, invalid Flutter configuration values can reach native code without a Dart-level error.
Examples:
Swift and Kotlin already reject these values earlier. Flutter did not.
Changes
Added missing Flutter configuration models
LLMConfigurationSTTConfigurationAdded missing validation
validate()forTTSConfigurationEnforced validation before native calls
generate()/generateStream()hit FFIExported config types publicly
Preserved validation errors at the public API layer
RunAnywhere.generate()soSDKError.validationFailed(...)is rethrown instead of being masked as a generic generation failureFiles touched
sdk/runanywhere-flutter/packages/runanywhere/lib/features/llm/llm_configuration.dartsdk/runanywhere-flutter/packages/runanywhere/lib/features/stt/stt_configuration.dartsdk/runanywhere-flutter/packages/runanywhere/lib/features/tts/system_tts_service.dartsdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_llm.dartsdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_stt.dartsdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_tts.dartsdk/runanywhere-flutter/packages/runanywhere/lib/public/runanywhere.dartsdk/runanywhere-flutter/packages/runanywhere/lib/runanywhere.dartExpected behavior after this PR
Invalid values now fail in Dart first with clear validation errors, for example:
Temperature must be between 0 and 2.0Max tokens must be between 1 and context lengthSample rate must be between 1 and 48000 HzSpeaking rate must be between 0.5 and 2.0Notes
Testing
dart format,dart analyze, or Flutter tests in this shell becausedart/flutterwere not available onPATHSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Greptile Summary
This PR adds Dart-side configuration validation for LLM, STT, and TTS components in the Flutter SDK, bringing it to parity with the Swift and Kotlin SDKs. Invalid parameters (e.g., negative temperature, out-of-range sample rates) now fail with clear
SDKError.validationFailedmessages before crossing the FFI boundary into C++.LLMConfigurationandSTTConfigurationmodels implementingComponentConfigurationwithvalidate()methods matching Swift/Kotlin rangesvalidate()implementation to the existingTTSConfigurationclass (speakingRate, pitch, volume)DartBridgeLLM.generate(),DartBridgeLLM.generateStream(),DartBridgeSTT.transcribe(), andDartBridgeTTS.synthesize()before FFI callsRunAnywhere.generate()to rethrowSDKErrorinstead of masking it as a generic generation failurecontextLength: 32768when validating, which means themaxTokens <= contextLengthcheck will always pass for values under 32768 regardless of the actual model's context window — this weakens the intended protectionConfidence Score: 4/5
dart_bridge_llm.dart— the hardcodedcontextLength: 32768in_validateGenerationParametersmakes themaxTokens <= contextLengthcheck effectively a no-op for realistic values.Important Files Changed
on SDKError { rethrow; }to prevent validation errors from being masked as generic generation failures. Correct and necessary fix.show TTSConfigurationto avoid exporting the entire system_tts_service barrel.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Public API Call] --> B{Which component?} B -->|generate / generateStream| C[DartBridgeLLM] B -->|transcribe| D[DartBridgeSTT] B -->|synthesize| E[DartBridgeTTS] C --> F[LLMConfiguration.validate] D --> G[STTConfiguration.validate] E --> H[TTSConfiguration.validate] F -->|Invalid| I[Throw SDKError.validationFailed] G -->|Invalid| I H -->|Invalid| I F -->|Valid| J[FFI Call to C++] G -->|Valid| J H -->|Valid| J I --> K{Caller} K -->|RunAnywhere.generate| L["on SDKError { rethrow }"] K -->|Other methods| M[Propagates naturally]Prompt To Fix All With AI
Last reviewed commit: 115e330