Skip to content

Conversation

Pouyanpi
Copy link
Collaborator

Description

Fix URL joining in NIM jailbreak detection to handle base URLs with/without trailing slashes and classification paths with/without leading slashes.

  • Add join_nim_url() helper function
  • Update jailbreak_nim_request() to use normalized URL joining
  • Add comprehensive tests covering all slash combinations

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes URL joining logic in NIM jailbreak detection to properly handle base URLs and classification paths with various combinations of trailing and leading slashes.

  • Introduces a new join_nim_url() helper function to normalize URL joining behavior
  • Updates jailbreak_nim_request() to use the new URL joining function instead of direct urljoin
  • Adds comprehensive test coverage for all possible slash combinations in URL paths

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
nemoguardrails/library/jailbreak_detection/request.py Adds join_nim_url() helper function and updates jailbreak_nim_request() to use normalized URL joining
tests/test_jailbreak_request.py Updates tests to use new helper function and adds comprehensive test cases for all slash combinations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Pouyanpi Pouyanpi mentioned this pull request Aug 22, 2025
4 tasks
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.05%. Comparing base (a5e8b4c) to head (76c6ade).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
...oguardrails/library/jailbreak_detection/request.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1346      +/-   ##
===========================================
+ Coverage    71.04%   71.05%   +0.01%     
===========================================
  Files          162      162              
  Lines        16398    16402       +4     
===========================================
+ Hits         11650    11655       +5     
+ Misses        4748     4747       -1     
Flag Coverage Δ
python 71.05% <83.33%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...oguardrails/library/jailbreak_detection/request.py 18.57% <83.33%> (+6.45%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@erickgalinkin erickgalinkin left a comment

Choose a reason for hiding this comment

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

LGTM

@Pouyanpi Pouyanpi added this to the v0.17.0 milestone Aug 28, 2025
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.

3 participants