Skip to content

test: add streaming coverage for per-tool rails - #2443

Merged
tanushriya910 merged 2 commits into
developfrom
test/per-tool-regex-streaming-coverage
Oct 10, 2026
Merged

tanushriya910 merged 2 commits into
developfrom
test/per-tool-regex-streaming-coverage

Conversation

@tanushriya910

@tanushriya910 tanushriya910 commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Adds streaming coverage for per-tool regex rails that the existing tests did not exercise:

  • Multiple streamed tool calls: TestStreamingPerToolCallRegex gains a test where only the second of two streamed calls matches the pattern (the stream is blocked with one tool_output_rails violation and no tool call leaks), and one where two clean calls both stream through in order.
  • Streamed tool results: new TestStreamingPerToolResultRegex. A result matching the pattern blocks the stream with a tool_input_rails violation before the model is called, and a clean result streams through. Tool results were previously only covered non-streaming.

The streaming path is the same for every per-tool rail: stream_async buffers the tool calls, then hands them to the same are_tool_calls_safe / are_tool_results_safe used in generation, which dispatch any per-tool rail identically. So these tests use the regex rail as a deterministic stand-in for that path rather than repeating it per rail type.

Related Issue(s)

  • NGUARD-985

Verification

  • make test: all passing.

AI Assistance

  • No AI tools were used.
  • AI tools were used; a human reviewed and can explain every change (tool: ___).

Checklist

  • I've read the CONTRIBUTING guidelines.
  • This PR links to a triaged issue assigned to me.
  • My PR title follows the project commit convention.
  • I've updated the documentation if applicable.
  • I've added tests if applicable.
  • I've noted any verification beyond CI and any checks I couldn't run.
  • I did not update generated changelog files manually.
  • I addressed all CodeRabbit, Greptile, and other review comments, or replied with why no change is needed.
  • @mentions of the person or team responsible for reviewing proposed changes.

Summary by CodeRabbit

  • Tests
    • Expanded coverage for streaming tool-call and tool-result checks, including cases where matching content is blocked and nonmatching content continues streaming.

@github-actions github-actions Bot added status: needs triage New issues that have not yet been reviewed or categorized. size: M labels Oct 8, 2026
@tanushriya910 tanushriya910 changed the title test: add streaming coverage for per-tool regex rails with multiple c… test: add streaming coverage for per-tool rails Oct 8, 2026
@tanushriya910 tanushriya910 added status: triaged Triaged by a maintainer; eligible for automated review (CodeRabbit/Greptile). and removed status: needs triage New issues that have not yet been reviewed or categorized. labels Oct 8, 2026
@tanushriya910
tanushriya910 marked this pull request as ready for review October 8, 2026 19:22
@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Low impact] This test-only PR appears safe to merge.

Summary

Adds streaming tests for per-tool regex rails and shares the helpers that build tool calls and conversations.

  • Tests a matching call at each position in a three-call stream.
  • Checks that three clean calls return in order.
  • Covers blocked and clean streamed tool results, including stopping before the model is called.
  • No new actionable issues found. Tests were not run during this review.

Reviews (2) · Last reviewed commit: "test: parametrize multi-call streaming t..." · Reviewed by Greptile

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA-NeMo/Guardrails/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: f15ef88d-031f-40ea-97fd-f85d636e28ab
📥 Commits

Reviewing files that changed from the base of the PR and between 9f793de and cb1b6d5.

📒 Files selected for processing (2)
  • tests/guardrails/test_per_tool_regex_rails_iorails.py
  • tests/guardrails/test_tool_rails_iorails.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The pull request adds streaming tests for per-tool regex rails. It also adds an SSE helper for multiple tool calls and moves tool-result conversation construction to a module-level helper.

Changes

Per-tool regex streaming tests

Layer / File(s) Summary
Tool-call streaming coverage
tests/guardrails/test_per_tool_regex_rails_iorails.py, tests/guardrails/test_tool_rails_iorails.py
Tests cover a regex match in a later tool call and multiple nonmatching calls. The SSE helper builds chunks for indexed tool calls.
Tool-result streaming coverage
tests/guardrails/test_per_tool_regex_rails_iorails.py
Tests use a module-level conversation helper. Streaming cases check matching and nonmatching tool-result content.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: tgasser-nv

Merge Risk: ⚪ Minimal · up to cb1b6

The added streaming tests cover the intended tool-call and tool-result behavior. No issue identified here prevents merging after normal checks.

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Test Results For Major Changes ✅ Passed The PR adds test coverage only: 86 insertions and 14 deletions in two test files. This is a minor change. The description also includes testing information: “make test: all passing,” plus the covered …
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding streaming test coverage for per-tool rails.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@tgasser-nv tgasser-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good just a couple of nits

Comment thread tests/guardrails/test_per_tool_regex_rails_iorails.py Outdated
Comment thread tests/guardrails/test_tool_rails_iorails.py Outdated
@tanushriya910
tanushriya910 merged commit 9a051dc into develop Oct 10, 2026
17 checks passed
@tanushriya910
tanushriya910 deleted the test/per-tool-regex-streaming-coverage branch October 10, 2026 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: M status: triaged Triaged by a maintainer; eligible for automated review (CodeRabbit/Greptile).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants