Skip to content

Conversation

@Pouyanpi
Copy link
Collaborator

This pull request makes several updates to the nemoguardrails/rails/llm/config.py file, focusing on improving deprecation handling, updating field configurations, and enhancing clarity in the codebase. The changes include replacing deprecated warnings with silent migrations, modifying field types, and cleaning up imports.

Deprecation handling improvements:

  • Replaced the deprecated warning for the remove_thinking_traces field with a silent migration to the remove_reasoning_traces field in the _migrate_thinking_traces method. This simplifies the user experience by automatically propagating values without issuing warnings.

Field configuration updates:

  • Updated the remove_thinking_traces field to include a deprecated attribute in its description, improving clarity on its deprecation status.
  • Changed the apply_to_reasoning_traces field in the OutputRails class from bool to Optional[bool] to allow for more flexible configurations.

Code cleanup:

  • Replaced the unused ValidationError import with Field to align with the updated field definitions.

@Pouyanpi Pouyanpi added this to the v0.14.0 milestone May 14, 2025
@Pouyanpi Pouyanpi requested a review from Copilot May 14, 2025 09:44
@Pouyanpi Pouyanpi self-assigned this May 14, 2025
@Pouyanpi Pouyanpi added the enhancement New feature or request label May 14, 2025
@Pouyanpi Pouyanpi changed the title refactor(config): update deprecated field handling refactor(config): update deprecated field handling for remove_thinking_traces May 14, 2025
Copy link

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 pull request refactors the configuration handling for deprecated fields and updates field types to improve clarity and functionality.

  • Replaces the deprecated warning for remove_thinking_traces with a silent migration to remove_reasoning_traces.
  • Updates the remove_thinking_traces field description to mark it as deprecated and adjusts types for specific fields.
  • Cleans up unused imports and changes the type of apply_to_reasoning_traces from bool to Optional[bool].
Comments suppressed due to low confidence (1)

nemoguardrails/rails/llm/config.py:467

  • Ensure that the change to Optional[bool] for apply_to_reasoning_traces is accompanied by any necessary adjustments in downstream code, as the new type may affect how its value is interpreted.
apply_to_reasoning_traces: Optional[bool] = Field(

@Pouyanpi Pouyanpi merged commit 74ff456 into develop May 14, 2025
28 checks passed
@Pouyanpi Pouyanpi deleted the fix/rails-config-schema branch May 14, 2025 10:01
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.43%. Comparing base (343b759) to head (1369279).
Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1196      +/-   ##
===========================================
- Coverage    68.43%   68.43%   -0.01%     
===========================================
  Files          161      161              
  Lines        15945    15943       -2     
===========================================
- Hits         10912    10910       -2     
  Misses        5033     5033              
Flag Coverage Δ
python 68.43% <100.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
nemoguardrails/rails/llm/config.py 89.96% <100.00%> (-0.04%) ⬇️
🚀 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants