Repository navigation
fix(commons): report TTS options allocation failures as out-of-memory - #1147
ShryukGrandhi wants to merge 1 commit into
Conversation
rac_tts_options_from_proto now returns a rac_result_t instead of a bool, so its three call sites (the lifecycle one-shot, lifecycle stream and stream session ABIs) propagate RAC_ERROR_OUT_OF_MEMORY for a failed string copy instead of reporting RAC_ERROR_DECODING_ERROR. Malformed request bytes still fail in parsing with a decoding error. Fixes RunanywhereAI#1060
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 5 files. (1 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the main change: TTS allocation failures are reported as out-of-memory errors. Description check The description includes the change summary, bug-fix classification, testing details, affected label, checklist status, linked issue, and platform testing context. It is complete and aligned with the … Linked Issues check Issue #1060requires allocation failures fromrac_tts_options_from_prototo reach all three TTS call sites asRAC_ERROR_OUT_OF_MEMORY, while malformed requests remainRAC_ERROR_DECODING_ERROR. T…Out of Scope Changes check The changes stay within issue #1060. The header and adapter update the result contract. The three callers propagate the result. The CMake interposition and test-local allocation hooks support allocati…Full details: Description check
-
Explanation
The description includes the change summary, bug-fix classification, testing details, affected label, checklist status, linked issue, and platform testing context. It is complete and aligned with the template.
Full details: Linked Issues check
-
Explanation
Issue
#1060requires allocation failures fromrac_tts_options_from_prototo reach all three TTS call sites asRAC_ERROR_OUT_OF_MEMORY, while malformed requests remainRAC_ERROR_DECODING_ERROR. The adapter now returnsrac_result_tand maps string-copy failures toRAC_ERROR_OUT_OF_MEMORY.rac_tts_synthesize_lifecycle_proto,rac_tts_synthesize_stream_lifecycle_proto, andrac_tts_stream_start_protoreturn that result. The added ABI tests cover voice and language allocation failures across all three APIs and cover malformed request bytes withRAC_ERROR_DECODING_ERROR.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#1060. The header and adapter update the result contract. The three callers propagate the result. The CMake interposition and test-local allocation hooks support allocation-failure tests. The mock streaming operation enables coverage of the stream lifecycle path. No unrelated behavior or files are changed.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 5 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="core/src/foundation/rac_proto_adapters.cpp">
<violation number="1" location="core/src/foundation/rac_proto_adapters.cpp:299">
P3: The shared adapter contract still says null inputs return `false`, but this adapter now returns `RAC_ERROR_NULL_POINTER`. Update the file-level comment to describe result-code returns for this adapter.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| rac_tts_options_t* out) { | ||
| if (!out) | ||
| return false; | ||
| return RAC_ERROR_NULL_POINTER; |
There was a problem hiding this comment.
P3: The shared adapter contract still says null inputs return false, but this adapter now returns RAC_ERROR_NULL_POINTER. Update the file-level comment to describe result-code returns for this adapter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At core/src/foundation/rac_proto_adapters.cpp, line 299:
<comment>The shared adapter contract still says null inputs return `false`, but this adapter now returns `RAC_ERROR_NULL_POINTER`. Update the file-level comment to describe result-code returns for this adapter.</comment>
<file context>
@@ -293,19 +293,20 @@ bool rac_stt_result_to_proto(const rac_stt_result_t* in, ::runanywhere::v1::STTO
+ rac_tts_options_t* out) {
if (!out)
- return false;
+ return RAC_ERROR_NULL_POINTER;
*out = RAC_TTS_OPTIONS_DEFAULT;
out->language = nullptr;
</file context>
This branch has not been deployed
1 participant
Description
rac_tts_options_from_protocan only fail when it cannot allocate a string, because every caller passes a non-nullout. Its three call sites still reported that asRAC_ERROR_DECODING_ERROR("failed to convert TTSOptions"), so a real out-of-memory condition looked like malformed client input:rac_tts_synthesize_lifecycle_proto(tts_module.cpp)rac_tts_synthesize_stream_lifecycle_proto(tts_module.cpp)rac_tts_stream_start_proto(rac_tts_stream.cpp)The adapter now returns a
rac_result_tinstead of abool:RAC_ERROR_NULL_POINTERfor a nullout,RAC_ERROR_OUT_OF_MEMORYwhen the voice or language copy fails, andRAC_SUCCESSotherwise. The three call sites pass that code through. Only the return type changes. The cleanup and failure-output rules in the header comment stay as they were. Malformed request bytes are still rejected earlier, when the request is parsed, and still returnRAC_ERROR_DECODING_ERROR. This matches howrac_vlm_image_from_protoalready separates the two cases.No other code calls the adapter (I checked
core/,engines/andbindings/).Fixes #1060
Type of Change
Testing
git clang-format --diff HEAD -- corereports no changes to the touched lines)I extended
core/tests/test_nonllm_lifecycle_proto_abi.cpp. Whenrac_commonsis a static library, the test defines its ownrac_alloc/rac_free/rac_strdup, which keeps the linker from usingrac_memory.cpp's versions. The test'srac_strdupreturns NULL for one sentinel string. I put that sentinel intoTTSOptions.language_codeand, separately, intoTTSOptions.voice, and checked that all three ABIs returnRAC_ERROR_OUT_OF_MEMORY. Malformed request bytes still returnRAC_ERROR_DECODING_ERRORon all three. The mock TTS backend gained a trivialsynthesize_stream, so the lifecycle stream path gets as far as the options conversion. The interposition only builds whenrac_commonsis aSTATIC_LIBRARY, which is set incore/tests/CMakeLists.txt.Without the fix, the test reports 6 failures (all of the OOM checks). With it, all checks pass.
Labels
Commons- Changes to shared native code (core)Checklist
Summary by cubic
Reports out-of-memory conditions during TTS options conversion as
RAC_ERROR_OUT_OF_MEMORYinstead ofRAC_ERROR_DECODING_ERROR, so allocation failures are no longer indistinguishable from malformed client input.rac_tts_options_from_protonow returnsrac_result_t; its three call sites (lifecycle one-shot, lifecycle stream, and stream sessions) propagate that result instead of always reporting a decoding error.RAC_ERROR_DECODING_ERROR.rac_strdupand verify all three call sites reportRAC_ERROR_OUT_OF_MEMORY.Fixes #1060.
Written for commit c2b016f. Summary will update on new commits.
Summary by CodeRabbit