Skip to content

Commit f7c8ee2

Browse files
fix: address PR #475 review comments — 6 fixes across Flutter config validation
- **QEF-1 (greptile P1, `dart_bridge_llm.dart:390`)**: `_requireLoadedContextLength()` no longer throws `SDKError.validationFailed` when the loaded model lacks `contextLength` metadata. Falls back to `32768` (a generous ceiling) so generation isn't silently broken for models that were registered without a non-zero `contextLength` in their C struct. Return type stays `int` (non-nullable) so call sites don't need to change; `SDKError` import dropped. - **QEF-2 / QEF-5 / QEF-7 (greptile P1 + coderabbit Critical ×2, `llm_configuration.dart:30`)**: Removed the hardcoded 32 768 ceiling on `contextLength` from `LLMConfiguration.validate()`. The previous `contextLength > 32768` rejection blocked every generation attempt on modern large-context models (Llama 3.1 8B at 128K, Mistral v0.3 at 32K, etc.). Kept only `contextLength <= 0`; the existing `maxTokens <= contextLength` check provides the effective upper bound. Error message updated to "Context length must be greater than 0". - **QEF-3 (greptile P2, `dart_bridge_model_registry.dart:200`)**: The `model.contextLength ?? 0` pattern in `savePublicModel` is in fact correct — `public_types.ModelInfo.contextLength` IS nullable (`int?`), and the internal `RacModelInfoCStruct.contextLength` is non-nullable `int` with `0` as the "missing" sentinel that `_ffiModelToPublic` maps back to `null`. Added a doc comment at the adapter boundary explaining the null↔zero convention so the intent is obvious and the reverse conversion is linked. - **QEF-4 (coderabbit Major, `llm_configuration.dart:48`)**: Added `systemPrompt` length validation to `validate()`. Oversized prompts (a system prompt longer than the model's context window, measured roughly as `contextLength * 4` chars at ~4 chars/token) are now rejected before the FFI call with a descriptive error. Null `systemPrompt` is still allowed. - **QEF-6 (coderabbit Minor, `tts_configuration.dart:16`)**: Aligned the default `speakingRate` from `0.5` to `1.0` so `const TTSConfiguration()` matches `dart_bridge_tts.synthesize()`'s default rate. The `validate()` range (0.5–2.0) still admits 1.0, no range change needed. - **QEF-8 (coderabbit Major, `dart_bridge_model_assignment.dart:257` + `dart_bridge_model_registry.dart:1040`)**: `_structToModelInfo` now propagates `contextLength` into the returned `ModelInfo`, so models loaded via the assignment/discovery flow carry the metadata that the LLM bridge's `_requireLoadedContextLength()` relies on. Added the corresponding `contextLength` field to `RacModelInfoStruct` to match the C layout read by this path. - **Cleanup**: removed 4 redundant `!` operators on `systemPromptPtr` in `dart_bridge_llm.dart` (lines 532, 599, 668, 704) and wrapped two non-awaited `controller.close()` calls in `unawaited(...)` (lines 357, 361) so `flutter analyze` reports a clean run on the modified files. **Invalid / superseded by code evolution:** none (all comments addressed). **Verification:** `flutter analyze` on the five modified files reports "No issues found!" with Flutter 3.38.3 / Dart 3.10.1. The older `dart analyze` (Dart 2.17.3, pre-Dart 3) still reports pre-existing "Future isn't a type" cascades because the repo requires Dart >=3.0.0 — that's an environment issue, not a code issue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ee86d2e commit f7c8ee2

5 files changed

Lines changed: 35 additions & 18 deletions

File tree

sdk/runanywhere-flutter/packages/runanywhere/lib/features/llm/llm_configuration.dart

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ class LLMConfiguration implements ComponentConfiguration {
2727

2828
@override
2929
void validate() {
30-
if (contextLength <= 0 || contextLength > 32768) {
30+
if (contextLength <= 0) {
3131
throw SDKError.validationFailed(
32-
'Context length must be between 1 and 32768',
32+
'Context length must be greater than 0',
3333
);
3434
}
3535

@@ -44,5 +44,15 @@ class LLMConfiguration implements ComponentConfiguration {
4444
'Max tokens must be between 1 and context length',
4545
);
4646
}
47+
48+
// Guard against clearly oversized prompts (chars) — a system prompt larger
49+
// than the model's context window (in chars) is clearly invalid.
50+
// Uses ~4 chars per token as a generous char-level bound.
51+
final prompt = systemPrompt;
52+
if (prompt != null && prompt.length > contextLength * 4) {
53+
throw SDKError.validationFailed(
54+
"systemPrompt length (${prompt.length} chars) exceeds the model's context window",
55+
);
56+
}
4757
}
4858
}

sdk/runanywhere-flutter/packages/runanywhere/lib/features/tts/tts_configuration.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class TTSConfiguration implements ComponentConfiguration {
1313
const TTSConfiguration({
1414
this.voice = 'system',
1515
this.language = 'en-US',
16-
this.speakingRate = 0.5,
16+
this.speakingRate = 1.0,
1717
this.pitch = 1.0,
1818
this.volume = 1.0,
1919
this.audioFormat = 'pcm',

sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_llm.dart

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import 'dart:isolate'; // Keep for non-streaming generation
1919

2020
import 'package:ffi/ffi.dart';
2121
import 'package:runanywhere/features/llm/llm_configuration.dart';
22-
import 'package:runanywhere/foundation/error_types/sdk_error.dart';
2322
import 'package:runanywhere/foundation/logging/sdk_logger.dart';
2423
import 'package:runanywhere/native/ffi_types.dart';
2524
import 'package:runanywhere/native/platform_loader.dart';
@@ -355,11 +354,11 @@ class DartBridgeLLM {
355354
controller.add(message);
356355
} else if (message is _StreamingMessage) {
357356
if (message.isComplete) {
358-
controller.close();
357+
unawaited(controller.close());
359358
receivePort.close();
360359
} else if (message.error != null) {
361360
controller.addError(StateError(message.error!));
362-
controller.close();
361+
unawaited(controller.close());
363362
receivePort.close();
364363
}
365364
}
@@ -389,13 +388,9 @@ class DartBridgeLLM {
389388

390389
int _requireLoadedContextLength() {
391390
final contextLength = _loadedContextLength;
392-
if (contextLength != null && contextLength > 0) {
393-
return contextLength;
394-
}
395-
396-
throw SDKError.validationFailed(
397-
'Loaded model is missing context length metadata for maxTokens validation',
398-
);
391+
// Fall back to a generous ceiling when registry metadata is absent,
392+
// so generation is not blocked for models without explicit contextLength.
393+
return (contextLength != null && contextLength > 0) ? contextLength : 32768;
399394
}
400395

401396
void _validateGenerationParameters({
@@ -534,7 +529,7 @@ void _streamingIsolateEntry(_StreamingIsolateParams params) {
534529
// Set systemPrompt if provided
535530
if (params.systemPrompt != null && params.systemPrompt!.isNotEmpty) {
536531
systemPromptPtr = params.systemPrompt!.toNativeUtf8();
537-
optionsPtr.ref.systemPrompt = systemPromptPtr!;
532+
optionsPtr.ref.systemPrompt = systemPromptPtr;
538533
} else {
539534
optionsPtr.ref.systemPrompt = nullptr;
540535
}
@@ -601,7 +596,7 @@ void _streamingIsolateEntry(_StreamingIsolateParams params) {
601596
calloc.free(promptPtr);
602597
calloc.free(optionsPtr);
603598
if (systemPromptPtr != null) {
604-
calloc.free(systemPromptPtr!);
599+
calloc.free(systemPromptPtr);
605600
}
606601
_isolateSendPort = null;
607602
}
@@ -670,7 +665,7 @@ _IsolateGenerationResult _generateInIsolate(
670665
// Set systemPrompt if provided
671666
if (systemPrompt != null && systemPrompt.isNotEmpty) {
672667
systemPromptPtr = systemPrompt.toNativeUtf8();
673-
optionsPtr.ref.systemPrompt = systemPromptPtr!;
668+
optionsPtr.ref.systemPrompt = systemPromptPtr;
674669
} else {
675670
optionsPtr.ref.systemPrompt = nullptr;
676671
}
@@ -706,7 +701,7 @@ _IsolateGenerationResult _generateInIsolate(
706701
calloc.free(optionsPtr);
707702
calloc.free(resultPtr);
708703
if (systemPromptPtr != null) {
709-
calloc.free(systemPromptPtr!);
704+
calloc.free(systemPromptPtr);
710705
}
711706
}
712707
}

sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_model_assignment.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ class DartBridgeModelAssignment {
253253
framework: struct.ref.framework,
254254
source: struct.ref.source,
255255
sizeBytes: struct.ref.sizeBytes,
256+
contextLength: struct.ref.contextLength,
256257
downloadURL: struct.ref.downloadURL != nullptr
257258
? struct.ref.downloadURL.toDartString()
258259
: null,

sdk/runanywhere-flutter/packages/runanywhere/lib/native/dart_bridge_model_registry.dart

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,15 @@ class DartBridgeModelRegistry {
188188
}
189189

190190
try {
191-
// Convert public ModelInfo to FFI ModelInfo
191+
// Convert public ModelInfo to FFI ModelInfo.
192+
//
193+
// Nullable -> non-nullable at the adapter boundary:
194+
// public_types.ModelInfo.downloadSize and .contextLength are `int?`
195+
// (null means "unknown"), while the internal FFI ModelInfo uses
196+
// non-nullable `int` to mirror the C struct (which uses 0 as the
197+
// sentinel for "unset"). The `?? 0` here encodes that null -> 0
198+
// convention; the reverse conversion in `_ffiModelToPublic` maps
199+
// `> 0 ? value : null` back to public types.
192200
final ffiModel = ModelInfo(
193201
id: model.id,
194202
name: model.name,
@@ -1036,6 +1044,9 @@ base class RacModelInfoStruct extends Struct {
10361044
@Int64()
10371045
external int sizeBytes;
10381046

1047+
@Int32()
1048+
external int contextLength;
1049+
10391050
external Pointer<Utf8> downloadURL;
10401051
external Pointer<Utf8> localPath;
10411052
external Pointer<Utf8> version;

0 commit comments

Comments
 (0)