Skip to content

fix(commons): report tool-calling output allocation failures - #740

Open
shubhamsinnh wants to merge 1 commit into
RunanywhereAI:mainfrom
shubhamsinnh:bugfix/commons-toolcalling-alloc-failures
Open

fix(commons): report tool-calling output allocation failures#740
shubhamsinnh wants to merge 1 commit into
RunanywhereAI:mainfrom
shubhamsinnh:bugfix/commons-toolcalling-alloc-failures

Conversation

@shubhamsinnh

@shubhamsinnh shubhamsinnh commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Description

Several tool-calling output paths in tool_calling.cpp returned RAC_SUCCESS even when their required output allocation failed, while the matching non-empty paths already return RAC_ERROR_OUT_OF_MEMORY. This makes allocation failures indistinguishable from success:

  • rac_tool_call_parse_with_format — the parse-fallback path returned RAC_SUCCESS with a NULL clean_text when the fallback malloc failed.
  • rac_tool_call_format_prompt_with_format — the empty-definitions path returned RAC_SUCCESS with a NULL prompt when malloc(1) failed.
  • rac_tool_call_format_prompt_json_with_format — the empty-tools JSON path returned RAC_SUCCESS with a NULL prompt when malloc(1) failed.
  • rac_tool_call_definitions_to_json — the empty-definitions path returned RAC_SUCCESS with a NULL output when malloc(3) (the "[]" literal) failed.

Each now returns RAC_ERROR_OUT_OF_MEMORY on allocation failure, matching the existing non-empty-path behavior. No other behavior changes.

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactoring

Testing

  • Lint passes locally
  • Added/updated tests for changes (no new tests per repo guidance)

Local:

  • git diff --check — clean.
  • g++ -std=c++20 -fsyntax-only -I core/include -I core/src -I <scratch-nlohmann> core/src/features/llm/tool_calling.cpp — exit 0 (nlohmann single header downloaded locally; RAC_HAVE_PROTOBUF not defined, so the protobuf-generated tool_calling.pb.h section is skipped).
  • Full native build and core/scripts/lint-cpp.sh (clang-format) could not run on this Windows host; the pr-build.yml native build is the authoritative gate.

Platform-Specific Testing (check all that apply)

Swift SDK / iOS Sample:

  • Tested on iPhone (Simulator or Device) — not run (no macOS host)
  • Tested on iPad / Tablet — not run
  • Tested on Mac (macOS target) — not run

Kotlin SDK / Android Sample:

  • Tested on Android Phone (Emulator or Device) — not run (no Android toolchain on this host)
  • Tested on Android Tablet — not run

Flutter SDK / Flutter Sample:

  • Tested on iOS — not run
  • Tested on Android — not run

React Native SDK / React Native Sample:

  • Tested on iOS — not run
  • Tested on Android — not run

Web SDK / Web Sample:

  • Tested in Chrome (Desktop) — not run
  • Tested in Firefox — not run
  • Tested in Safari — not run
  • WASM backends load (LlamaCpp + ONNX) — not run
  • OPFS storage persistence verified (survives page refresh) — not run
  • Settings persistence verified (localStorage) — not run

Labels

Please add the appropriate label(s):

SDKs:

  • Swift SDK - Changes to Swift SDK (bindings/swift)
  • Kotlin SDK - Changes to Kotlin SDK (bindings/kotlin)
  • Flutter SDK - Changes to Flutter SDK (bindings/flutter)
  • React Native SDK - Changes to React Native SDK (bindings/react-native)
  • Web SDK - Changes to Web SDK (bindings/web)
  • Commons - Changes to shared native code (core)

Sample Apps:

  • Flutter Sample - Changes to Flutter example app (bindings/flutter/example)
  • React Native Sample - Changes to React Native example app (bindings/react-native/example)
  • Minimal Examples - Changes to an in-repo SDK harness (bindings/{swift,kotlin,web}/example)

The iOS, Android, Web, and Electron consumer apps live in their own
repositories (RunanywhereAI/runanywhere-{ios,android,web,electron}) — open
those PRs there.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Documentation updated (if needed) — no public API contract change

Screenshots

Attach relevant UI screenshots for changes (if applicable):

  • Mobile (Phone)
  • Tablet / iPad
  • Desktop / Mac

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of memory allocation failures when processing fallback text, empty prompts, and empty tool definitions.
    • Operations now report an out-of-memory error instead of incorrectly succeeding with empty output.

@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: dc632c6d-28af-44d5-8456-c15be09d6d23

📥 Commits

Reviewing files that changed from the base of the PR and between 101bf2f and 58f23a4.

📒 Files selected for processing (1)
  • core/src/features/llm/tool_calling.cpp

Included review availability: Your plan includes up to 10 reviews per rolling hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The LLM tool-calling helpers now detect allocation failures when creating fallback clean text, empty prompts, and empty tool-definition JSON. They return RAC_ERROR_OUT_OF_MEMORY instead of continuing with null output buffers.

Changes

LLM allocation error handling

Layer / File(s) Summary
Fallback output allocation checks
core/src/features/llm/tool_calling.cpp
The parsing and formatting helpers now return RAC_ERROR_OUT_OF_MEMORY when fallback output allocation fails.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 58f23

This localized change makes four allocation-failure paths return an out-of-memory error instead of reporting success with null output; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: sanchitmonga22

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the allocation-failure fix, identifies affected paths, documents testing limits, and completes the relevant template sections.
Title check ✅ Passed The title clearly and concisely describes the main change: reporting tool-calling output allocation failures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@shubhamsinnh
shubhamsinnh marked this pull request as ready for review August 18, 2026 15:35
@MohammedAlkindi

Copy link
Copy Markdown

The four public paths look right. The same shape survives in the static bool helpers that feed them: extract_json_value:619, extract_tool_name_and_args:940,1030,1042,1049, parse_lfm2_format:1373 and parse_default_format:1575 each malloc, guard with if (ptr) { ... }, and then report success.

Two are sharper. At 1373 and 1575 the allocation is *out_clean_text but the function ends return *out_tool_name != nullptr;, so a failed clean_text allocation returns true with a NULL clean_text. That is the same indistinguishable-from-success you are fixing above, one layer down.

Line numbers are against the base, all below your first hunk. Read only, no C++ build here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants