Skip to content

Conversation

Pouyanpi
Copy link
Collaborator

Description

Previously, messages of type "tool" were not distinctly labeled in the LoggingCallbackHandler output, causing them to be grouped under "System".
This change adds explicit handling for "tool" messages, labeling them as "Tool" in the logs for improved clarity and debugging.

@Pouyanpi Pouyanpi added this to the v0.17.0 milestone Sep 24, 2025
@Pouyanpi Pouyanpi self-assigned this Sep 24, 2025
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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 a logging issue where tool messages were incorrectly labeled as "System" messages in the LoggingCallbackHandler output. The change adds explicit handling for "tool" type messages to improve debugging clarity.

  • Adds "Tool" label for messages with type "tool" in the logging callback handler
  • Includes comprehensive test coverage to verify the new tool message labeling functionality

Reviewed Changes

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

File Description
nemoguardrails/logging/callbacks.py Adds conditional logic to label tool messages as "Tool" instead of defaulting to "System"
tests/test_callbacks.py Adds test case with mock verification to ensure tool messages are properly labeled in logging output

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

Copy link
Collaborator

@tgasser-nv tgasser-nv left a comment

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 cleanups and suggestion on using caplog instead of patching logging ourselves before merging.

Previously, messages of type "tool" were not distinctly labeled in the
LoggingCallbackHandler output, causing them to be grouped under
"System".
This change adds explicit handling for "tool" messages, labeling them as
"Tool" in the logs for improved clarity and debugging.
@Pouyanpi Pouyanpi force-pushed the fix/logging-tool-type branch from 27318b6 to 08629b5 Compare September 25, 2025 06:52
@Pouyanpi Pouyanpi merged commit ccbc6ca into develop Sep 25, 2025
12 of 14 checks passed
@Pouyanpi Pouyanpi deleted the fix/logging-tool-type branch September 25, 2025 16:18
Pouyanpi added a commit that referenced this pull request Sep 26, 2025
Previously, messages of type "tool" were not distinctly labeled in the
LoggingCallbackHandler output, causing them to be grouped under
"System".
This change adds explicit handling for "tool" messages, labeling them as
"Tool" in the logs for improved clarity and debugging.
Pouyanpi added a commit that referenced this pull request Oct 1, 2025
Previously, messages of type "tool" were not distinctly labeled in the
LoggingCallbackHandler output, causing them to be grouped under
"System".
This change adds explicit handling for "tool" messages, labeling them as
"Tool" in the logs for improved clarity and debugging.
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