-
Notifications
You must be signed in to change notification settings - Fork 524
fix(topic_safety): handle InternalEvent objects in topic safety actions for Colang 2.0 #1335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
…ns for Colang 2.0 Convert InternalEvent objects to dictionary format before passing to to_chat_messages() to prevent 'InternalEvent' object is not subscriptable TypeError when using topic safety with Colang 2.0 runtime.
There was a problem hiding this 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 a TypeError
that occurs when using topic safety rails with Colang 2.0 configurations. The issue arises because Colang 2.0 passes InternalEvent
objects to the topic_safety_check_input
action, but the existing code expected dictionary events.
- Added conversion logic to handle both
InternalEvent
objects and dictionary events - Maintained backward compatibility with existing dictionary-based event handling
- Added comprehensive regression test to prevent future occurrences
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
nemoguardrails/library/topic_safety/actions.py |
Added conversion logic to transform InternalEvent objects to dictionary format before passing to to_chat_messages() |
tests/test_topic_safety_internalevent.py |
Added regression test that verifies the fix handles InternalEvent objects without throwing TypeError |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
# convert InternalEvent objects to dictionary format for compatibility with to_chat_messages | ||
dict_events = [] | ||
for event in events: | ||
if hasattr(event, "name") and hasattr(event, "arguments"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using isinstance
check instead of hasattr
for more robust type checking. This would be more explicit about the expected type: isinstance(event, InternalEvent)
if hasattr(event, "name") and hasattr(event, "arguments"): | |
if isinstance(event, InternalEvent): |
Copilot uses AI. Check for mistakes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1335 +/- ##
===========================================
+ Coverage 70.70% 70.88% +0.18%
===========================================
Files 161 161
Lines 16312 16319 +7
===========================================
+ Hits 11533 11568 +35
+ Misses 4779 4751 -28
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Description
Fixes
TypeError
when using topic safety rails with Colang 2.0 configurations.Problem
When running topic safety checks in Colang 2.0, the
topic_safety_check_input
action would fail with:TypeError: 'InternalEvent' object is not subscriptable
This occurred because Colang 2.0 passes
InternalEvent
objects to the action, but the code was trying to pass them directly toto_chat_messages()
which expects dictionary events.Solution
InternalEvent
objects to dictionary format before passing toto_chat_messages()
Testing
Related PR(s)
#1289