feat(iorails): Tool-calling - non-streaming tool calling to main LLM#2016
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR wires non-streaming tool-calling through the IORails pipeline: tool definitions flow from
|
| Filename | Overview |
|---|---|
| nemoguardrails/guardrails/iorails.py | Adds _serialize_tool_calls and _build_assistant_message helpers; is_tool_call_only gate and output-rail skip logic are correct and match LLMRails semantics |
| nemoguardrails/guardrails/model_engine.py | _parse_chat_completion updated to parse tool calls via ChatMessage.from_dict; null-content handling is correct and well-tested |
| nemoguardrails/guardrails/guardrails_types.py | LLMMessage type alias widened from dict[str, str] to dict[str, Any] to accommodate None content and tool_calls list |
| tests/guardrails/test_iorails.py | New TestToolCalling class covers request forwarding, tool-call-only response assembly, output-rail skip, and mixed text+tool_calls path |
| tests/guardrails/test_model_engine.py | Existing tool-call rejection tests replaced with parsing tests; new cases cover parallel calls, reasoning alongside tool calls, and null-content validation |
| tests/guardrails/test_iorails_streaming.py | Verifies that tool definitions in llm_params are forwarded unchanged on the streaming path |
Reviews (7): Last reviewed commit: "Improve test coverage" | Re-trigger Greptile
📝 WalkthroughWalkthroughThis PR adds end-to-end support for OpenAI-style tool calling throughout the guardrails framework. The message type contract is broadened to accept arbitrary payloads, new typed configuration models for tool definitions and options are introduced with validation, the model engine is extended to parse tool-call responses and normalize content appropriately, and the IORails generation pipeline is integrated to forward tool-calling parameters, handle serialization, and conditionally run safety rails. ChangesOpenAI Tool Calling Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@nemoguardrails/guardrails/iorails.py`:
- Around line 411-428: The code determines is_tool_call_only from
response.tool_calls and response_text before injecting reasoning_content, which
allows unmoderated text to slip past rails; move the reasoning_content injection
so response_text is updated before computing is_tool_call_only and before
calling self.rails_manager.is_output_safe (or alternatively, ensure that if
reasoning_content is present you still run is_output_safe on the combined text);
update the flow around is_tool_call_only, response_text,
rails_manager.is_output_safe, and the subsequent return via
_build_assistant_message so the final assistant message (including <think>
reasoning) always goes through output rails and metrics recording when blocked.
In `@nemoguardrails/rails/llm/options.py`:
- Around line 224-232: The ToolCallingOptions docstring is stale about
consumption; update the class docstring for ToolCallingOptions (and mention
GenerationOptions.tool_calling) to state that these options are now forwarded
into the main LLM call (see iorails.py where tool-calling is forwarded into the
main LLM), removing the “not yet consumed by any engine” sentence and replacing
it with a brief note that engines currently receive/forward these options into
the primary LLM invocation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 83ec6fc4-03b8-4c97-8d5c-2d867a5684bc
📒 Files selected for processing (7)
nemoguardrails/guardrails/guardrails_types.pynemoguardrails/guardrails/iorails.pynemoguardrails/guardrails/model_engine.pynemoguardrails/rails/llm/options.pytests/guardrails/test_iorails.pytests/guardrails/test_model_engine.pytests/rails/llm/test_options.py
Pouyanpi
left a comment
There was a problem hiding this comment.
LGTM overall, the response-side tool call handling is clean 👍🏻 One concern: ToolCallingOptions please see my comment below.
Description
Route tool-calling
llm_paramsfields through to ModelEngine and serialize to an OpenAI-compatible format on main LLM inference.Related Issue(s)
NGUARD-820
Test Plan
Pre-commit
Unit-test
Integration test with Chat (uses NVCF models)
Local integration test using
e2e_main_llm.pywith OpenAI gpt-4o and mocked function call.Checklist
Summary by CodeRabbit
Release Notes
New Features
Tests