SDK API parity, telemetry field fixes, LLM token-count fixes, and new rcli commands#569
SDK API parity, telemetry field fixes, LLM token-count fixes, and new rcli commands#569Siddhesh2377 wants to merge 15 commits into
Conversation
…and add rcli rag/lora commands
…dels; fix LLM result UTF-8 sanitization and unary prompt_eval
… through vtable generate_stream
…ity POST asserting backend 2xx
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds inference timing and token metrics, expands telemetry serialization and validation, introduces CLI RAG, benchmarking, LoRA, and authenticated telemetry flows, aligns SDK APIs across platforms, adds public API documentation, and adds Android thinking controls. ChangesInference and telemetry
CLI workflows
Cross-platform SDK surfaces
Android chat controls
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
…sponse, mirrors Swift)
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/runanywhere-cli/src/commands/cmd_bench.cpp`:
- Line 1: Make the command files thin by moving orchestration into single
commons entry points: extract the load/warmup/measure/unload flow from run_bench
and its *_trial/*_generate helpers into a benchmark entry point, consolidate
run_rag_query’s session create/ingest/query/destroy sequence into one RAG-query
entry point, and fold model loading into run_lora_apply’s commons LoRA-apply
path. Keep each command responsible only for parsing, bootstrap, one commons
invocation, and rendering.
- Around line 156-170: Update available_ram_bytes() to obtain memory information
through rac_platform_adapter_t::get_memory_info instead of directly reading
/proc/meminfo. Reuse the desktop adapter’s cross-platform memory data and return
the available-memory value in bytes, preserving a safe fallback when the adapter
call fails.
In `@sdk/runanywhere-commons/tests/CMakeLists.txt`:
- Line 580: Update the target_compile_features declaration for
test_telemetry_extraction to require cxx_std_20 instead of cxx_std_17, leaving
the target and visibility unchanged.
In
`@sdk/runanywhere-kotlin/src/main/kotlin/com/runanywhere/sdk/public/extensions/LLM/RunAnywhereWebSearchTool.kt`:
- Around line 192-196: Update the HTTP response handling around
client.newCall(request).execute() to replace runCatching/getOrDefault with
explicit try/catch that rethrows kotlinx.coroutines.CancellationException and
handles other exceptions by returning an empty string. Within the response
block, safely read the nullable response.body using a null-safe fallback to an
empty string.
In `@sdk/runanywhere-web/packages/core/src/Public/Extensions/RunAnywhere`+TTS.ts:
- Around line 196-214: Update synthesizeStreamAuto to validate
adapter.supportsLifecycleProtoTTS() instead of supportsProtoTTS() before calling
synthesizeLifecycleStream, matching the capability check used by synthesize and
ensuring lifecycle exports are available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01f484c6-df88-4a25-b871-e37dd145b458
⛔ Files ignored due to path filters (5)
sdk/runanywhere-commons/src/generated/proto/sdk_events.pb.ccis excluded by!**/generated/**sdk/runanywhere-commons/src/generated/proto/sdk_events.pb.his excluded by!**/generated/**sdk/runanywhere-flutter/packages/runanywhere/lib/generated/sdk_events.pb.dartis excluded by!**/generated/**sdk/runanywhere-kotlin/src/main/kotlin/com/runanywhere/sdk/generated/ai/runanywhere/proto/v1/GenerationEvent.ktis excluded by!**/generated/**sdk/runanywhere-swift/Sources/RunAnywhere/Generated/sdk_events.pb.swiftis excluded by!**/generated/**
📒 Files selected for processing (71)
CMakePresets.jsonengines/llamacpp/llamacpp_backend.cppengines/llamacpp/llamacpp_backend.hengines/llamacpp/rac_backend_llamacpp_register.cppengines/llamacpp/rac_llm_llamacpp.cppengines/llamacpp/rac_vlm_llamacpp.cppengines/mlx/rac_mlx_engine.cppengines/onnx/onnx_embedding_provider.cppengines/onnx/onnx_embedding_provider.hengines/onnx/rac_onnx_embeddings_register.cppexamples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/ui/screens/chat/ChatInputBar.ktexamples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/ui/screens/chat/ChatScreen.ktexamples/android/RunAnywhereAI/app/src/main/java/com/runanywhere/runanywhereai/ui/screens/chat/ChatViewModel.ktidl/sdk_events.protoscripts/validation/gates/deprecated_surface_allowlist.txtsdk/runanywhere-cli/CMakeLists.txtsdk/runanywhere-cli/src/app.cppsdk/runanywhere-cli/src/bootstrap.cppsdk/runanywhere-cli/src/bootstrap.hsdk/runanywhere-cli/src/commands/cmd_bench.cppsdk/runanywhere-cli/src/commands/cmd_lora.cppsdk/runanywhere-cli/src/commands/cmd_rag.cppsdk/runanywhere-cli/src/commands/commands.hsdk/runanywhere-cli/tests/CMakeLists.txtsdk/runanywhere-cli/tests/test_rcli_telemetry_live.cppsdk/runanywhere-commons/docs/PublicApiFlutter.dartsdk/runanywhere-commons/docs/PublicApiKotlin.ktsdk/runanywhere-commons/docs/PublicApiReactNative.tssdk/runanywhere-commons/docs/PublicApiSwift.swiftsdk/runanywhere-commons/docs/PublicApiWeb.tssdk/runanywhere-commons/docs/architecture.htmlsdk/runanywhere-commons/docs/md/flutter.mdsdk/runanywhere-commons/docs/md/kotlin.mdsdk/runanywhere-commons/docs/md/react-native.mdsdk/runanywhere-commons/docs/md/swift.mdsdk/runanywhere-commons/docs/md/web.mdsdk/runanywhere-commons/include/rac/backends/rac_llm_llamacpp.hsdk/runanywhere-commons/include/rac/features/llm/rac_llm_service.hsdk/runanywhere-commons/include/rac/features/llm/rac_llm_types.hsdk/runanywhere-commons/include/rac/features/vlm/rac_vlm_types.hsdk/runanywhere-commons/include/rac/infrastructure/telemetry/rac_telemetry_types.hsdk/runanywhere-commons/src/features/embeddings/embeddings_module.cppsdk/runanywhere-commons/src/features/llm/llm_module.cppsdk/runanywhere-commons/src/features/llm/rac_llm_service.cppsdk/runanywhere-commons/src/features/llm/structured_output.cppsdk/runanywhere-commons/src/features/lora/rac_lora_service.cppsdk/runanywhere-commons/src/features/rag/rac_rag_proto_abi.cppsdk/runanywhere-commons/src/features/vlm/vlm_module.cppsdk/runanywhere-commons/src/infrastructure/telemetry/telemetry_json.cppsdk/runanywhere-commons/src/infrastructure/telemetry/telemetry_manager.cppsdk/runanywhere-commons/tests/CMakeLists.txtsdk/runanywhere-commons/tests/test_telemetry_extraction.cppsdk/runanywhere-flutter/packages/runanywhere/lib/public/runanywhere.dartsdk/runanywhere-kotlin/src/main/kotlin/com/runanywhere/sdk/public/extensions/LLM/RunAnywhereLoRA.ktsdk/runanywhere-kotlin/src/main/kotlin/com/runanywhere/sdk/public/extensions/LLM/RunAnywhereWebSearchTool.ktsdk/runanywhere-kotlin/src/main/kotlin/com/runanywhere/sdk/public/extensions/VLM/RunAnywhereVisionLanguage.ktsdk/runanywhere-react-native/packages/core/src/Public/Events/EventBus.tssdk/runanywhere-react-native/packages/core/src/Public/Extensions/Events/RunAnywhere+SDKEvents.tssdk/runanywhere-react-native/packages/core/src/Public/Extensions/LLM/RunAnywhere+LoRA.tssdk/runanywhere-react-native/packages/core/src/Public/Extensions/Models/RunAnywhere+ModelRegistry.tssdk/runanywhere-react-native/packages/core/src/Public/Extensions/Storage/RunAnywhere+Storage.tssdk/runanywhere-react-native/packages/core/src/Public/Extensions/VLM/RunAnywhere+VisionLanguage.tssdk/runanywhere-react-native/packages/core/src/Public/RunAnywhere.tssdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+RAG.swiftsdk/runanywhere-swift/Sources/RunAnywhere/Public/Extensions/RAG/RunAnywhere+RAG.swiftsdk/runanywhere-web/packages/core/src/Public/Extensions/RunAnywhere+FlatFacade.tssdk/runanywhere-web/packages/core/src/Public/Extensions/RunAnywhere+ModelRegistry.tssdk/runanywhere-web/packages/core/src/Public/Extensions/RunAnywhere+TTS.tssdk/runanywhere-web/packages/core/src/Public/Extensions/RunAnywhere+VAD.tssdk/runanywhere-web/packages/core/src/Public/RunAnywhere.tssdk/shared/proto-ts/src/sdk_events.ts
💤 Files with no reviewable changes (1)
- sdk/runanywhere-react-native/packages/core/src/Public/Extensions/Models/RunAnywhere+ModelRegistry.ts
…de count through vtable generate_stream" This reverts commit 607cb63.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
sdk/runanywhere-commons/src/features/llm/llm_module.cpp (3)
1669-1676: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSanitize extracted structured JSON before storing it.
Only
raw_outputis sanitized.validation.extracted_jsonis still assigned directly to protobuf fields, so invalid bytes embedded in extracted JSON can bypass the new boundary sanitization.Sanitize
validation.extracted_jsoneverywhere it is stored, includingset_json_output()andset_extracted_json().Proposed fix
- out->set_json_output(validation.extracted_json); + out->set_json_output(sanitize_utf8(validation.extracted_json)); ... - structured->set_extracted_json(validation.extracted_json); + structured->set_extracted_json(sanitize_utf8(validation.extracted_json));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/runanywhere-commons/src/features/llm/llm_module.cpp` around lines 1669 - 1676, Sanitize validation.extracted_json before every protobuf assignment in the structured-output validation flow. Update both set_extracted_json() and set_json_output() call sites to pass the sanitized value, while preserving the existing raw_output sanitization and validation behavior.
1577-1606: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject overlong and surrogate UTF-8 sequences.
The validator checks continuation bytes but accepts invalid scalar values such as
E0 80 80,ED A0 80, andF0 80 80 80. Those bytes can still be emitted into protobuf strings despite the new sanitization guarantee.Add the RFC 3629 boundary checks for
E0,ED,F0, andF4before appending a sequence.Proposed validation adjustment
bool ok = len > 0; for (size_t k = 1; ok && k < len; ++k) { ok = is_cont(i + k); } + if (ok && len == 3) { + const unsigned char c1 = static_cast<unsigned char>(in[i + 1]); + ok = !(c == 0xE0 && c1 < 0xA0) && + !(c == 0xED && c1 >= 0xA0); + } else if (ok && len == 4) { + const unsigned char c1 = static_cast<unsigned char>(in[i + 1]); + ok = !(c == 0xF0 && c1 < 0x90) && + !(c == 0xF4 && c1 > 0x8F); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/runanywhere-commons/src/features/llm/llm_module.cpp` around lines 1577 - 1606, Update sanitize_utf8 to enforce RFC 3629 boundary rules before appending multibyte sequences: reject E0 followed by 80–9F, ED followed by A0–BF, F0 followed by 80–8F, and F4 followed by 90–BF. Preserve the existing continuation-byte validation and replacement-character behavior for rejected sequences.
2308-2311: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not emit TTFT as
prompt_eval_time_ms.
stream_ttftmeasures time to the first token, butGenerationEvent.prompt_eval_time_msis defined as prompt-evaluation/prefill duration inidl/sdk_events.proto:418-437. This currently makes every streaming telemetry event report the wrong metric.Pass a separately measured prefill duration, or leave this field unset/zero when the backend does not provide one.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/runanywhere-commons/src/features/llm/llm_module.cpp` around lines 2308 - 2311, Update the streaming GenerationEvent construction near the existing stream_ttft argument so prompt_eval_time_ms no longer receives stream_ttft. Pass a separately measured prompt-evaluation/prefill duration when available; otherwise leave the field unset or set it to zero, while preserving stream_ttft for the TTFT metric.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@sdk/runanywhere-commons/src/features/llm/llm_module.cpp`:
- Around line 1669-1676: Sanitize validation.extracted_json before every
protobuf assignment in the structured-output validation flow. Update both
set_extracted_json() and set_json_output() call sites to pass the sanitized
value, while preserving the existing raw_output sanitization and validation
behavior.
- Around line 1577-1606: Update sanitize_utf8 to enforce RFC 3629 boundary rules
before appending multibyte sequences: reject E0 followed by 80–9F, ED followed
by A0–BF, F0 followed by 80–8F, and F4 followed by 90–BF. Preserve the existing
continuation-byte validation and replacement-character behavior for rejected
sequences.
- Around line 2308-2311: Update the streaming GenerationEvent construction near
the existing stream_ttft argument so prompt_eval_time_ms no longer receives
stream_ttft. Pass a separately measured prompt-evaluation/prefill duration when
available; otherwise leave the field unset or set it to zero, while preserving
stream_ttft for the TTFT metric.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4184c6db-a368-4de9-94e1-9bbd715e74e0
⛔ Files ignored due to path filters (2)
sdk/shared/proto-ts/dist/sdk_events.d.tsis excluded by!**/dist/**sdk/shared/proto-ts/dist/sdk_events.jsis excluded by!**/dist/**
📒 Files selected for processing (3)
engines/llamacpp/rac_llm_llamacpp.cppsdk/runanywhere-commons/src/features/llm/llm_module.cppsdk/runanywhere-commons/tests/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/runanywhere-commons/tests/CMakeLists.txt
… format web search tool
This branch collects a few weeks of SDK/commons work. The threads are separate but landed together, so here's what's in it and why.
SDK public API parity
Aligned the public API surface of Kotlin, React Native, Flutter, and Web to the Swift facade (the source of truth), keeping platform-genuine differences. Added flat
RunAnywhere.xaliases where an SDK hid a feature behind a capability object. Per-SDK usage docs added underdocs/md/. Web-search on RN/Flutter/Web andsetHfTokenon Web are deliberately left out — those SDKs route HTTP through native C++, so a JS/Dart HTTP tool fights the design.Telemetry: fill the missing fields
Several per-modality telemetry fields were arriving null. Fixed at the engine/commons level so every SDK benefits:
prompt_eval_time_ms(prefill was never populated on the unary result)total_tokens(real WordPiece count from the ONNX provider) andbatch_sizebase_model_id/adapter_id/adapter_size_byteson the failure path (they were dropped)query_token_count/context_tokensLLM token undercount (this one actually mattered)
tokens_per_secondandtokens_generatedwere under-reported by up to ~5x. The count came from streaming-callback invocations, which flush buffered chunks rather than one call per token. Now the unary path uses the decode loop's real count (the streaming path keeps the callback-based count to avoid a vtable ABI change that would ripple into every backend). Auditing the benchmark is what surfaced it — smollm2 reported 4.8 tok/s where the real number was ~45. Also addedsanitize_utf8at the proto string boundaries, since a model cut off mid-multibyte-character produced invalid UTF-8 that brokerac_llm_generate_protoserialization entirely.rcli: bench, rag, lora
rcli benchauto-benchmarks every installed model (LLM/STT/TTS/VLM), matching the Android benchmark screen: load, warmup, N trials, median.RAC_BACKEND_RAGturned on in the committed rcli presets (it also gates ONNX embeddings).rcli rag queryruns a full ingest + query through the RAG session ABI.rcli lora applyloads an LLM and attaches an adapter.Telemetry tests
test_telemetry_extraction(commons): drives SDKEvent protos through the extraction/routing/JSON pipeline against a mock sink and checks the per-modality endpoint + fields offline. Regression guard for every bug above, including the NaN-confidence one.test_rcli_telemetry_live --live(rcli): sends real authenticated telemetry per modality and asserts the backend returns 2xx. Opt-in — no-ops without--liveand creds, so CI stays green.Also
Android chat composer redesign (two-row layout + thinking toggle, from #560).
idl/sdk_events.protogainedGenerationEvent.prompt_eval_time_ms(field 34); all five language bindings regenerated.Verified on Linux: rcli builds and runs every modality end to end, telemetry confirmed landing in Supabase, both test suites pass.
Summary by CodeRabbit