Skip to content

feat: implement validation to forbid dialog rails with reasoning traces #1137

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

Merged
merged 5 commits into from
Apr 25, 2025

Conversation

Pouyanpi
Copy link
Collaborator

@Pouyanpi Pouyanpi commented Apr 22, 2025

Description

This PR implements a validator to ensure that reasoning traces cannot be enabled when dialog rails are present. The validator checks for both explicit dialog rails configuration and implicit dialog rails enabled through user messages, bot messages, and flows.

Impact

This change ensures that:

  1. Users cannot enable reasoning traces when dialog rails are present
  2. The validation covers all ways dialog rails can be enabled
  3. Clear error messages guide users to resolve configuration conflicts

Changes

  1. Added check_reasoning_traces_with_dialog_rails validator to RailsConfig class that:

    • Checks if any model has reasoning traces enabled
    • Verifies presence of dialog rails through:
      • Explicit configuration in rails.dialog
      • Implicit configuration through user_messages
      • Implicit configuration through bot_messages
      • Implicit configuration through flows
    • Raises a ValueError if reasoning traces are enabled while dialog rails are present
  2. Added test cases in test_config_validation.py to verify:

    • Error is raised when both reasoning traces and dialog rails are enabled
    • No error when only reasoning traces are present
    • No error when only dialog rails are present
    • Error is raised when reasoning traces are enabled with implicit dialog rails

Testing

Added comprehensive test coverage:

Technical Details

The validator is implemented as a root validator in the RailsConfig class that (must migrated as part of #967:

  • Takes pre-validated values as input
  • Checks model configurations for reasoning traces
  • Verifies dialog rails presence through multiple sources
  • Returns the values if validation passes
  • Raises an error if validation fails

Breaking Changes

None. This is a new validation that prevents invalid configurations while it maintains backward compatibility

TODO:

  • check for reasoning traces per dialog task

@Pouyanpi Pouyanpi self-assigned this Apr 22, 2025
@Pouyanpi Pouyanpi added the enhancement New feature or request label Apr 22, 2025
@Pouyanpi Pouyanpi added this to the v0.14.0 milestone Apr 22, 2025
@Pouyanpi Pouyanpi marked this pull request as draft April 22, 2025 14:15
@Pouyanpi Pouyanpi force-pushed the feat/reasoning-traces-validation branch from fba8078 to d8e90cd Compare April 22, 2025 15:50
Added a root validator in `RailsConfig` to ensure that reasoning traces
cannot be enabled when dialog rails are present, either explicitly or
implicitly. This includes checks for user messages, bot messages, and
flows.

Also added comprehensive test cases to validate this behavior:
- Explicit dialog rails
- Implicit dialog rails via user/bot messages or flows
- Scenarios without dialog rails to ensure reasoning traces work as
  expected.
@Pouyanpi Pouyanpi force-pushed the feat/reasoning-traces-validation branch from d8e90cd to a6f620f Compare April 23, 2025 10:52
- Fix validation to properly handle both dictionary and Model object cases
- Add proper handling of cases where some dialog rail tasks have dedicated models and others fall back to main model
- Improve error messages to be more user-friendly:
  * Specify which model has the issue (main model or specific task model)
  * Reference config.yml in error messages
  * Provide clear YAML configuration instructions
  * Include specific task name when relevant
- Update test cases to match new error messages and validation logic
- Add proper handling of implicit dialog rails activation through user/bot messages and flows

This change ensures that reasoning traces are properly disabled when dialog rails are present,
regardless of whether they are explicitly configured or implicitly activated through user/bot
messages or flows. The improved error messages make it easier for users to understand and fix
configuration issues in their YAML files.

add more tests
add tests to show the expected behavior of dialog tasks
@Pouyanpi Pouyanpi force-pushed the feat/reasoning-traces-validation branch from f484f15 to 6f8196e Compare April 24, 2025 08:21
@Pouyanpi Pouyanpi marked this pull request as ready for review April 24, 2025 08:24
@Pouyanpi Pouyanpi changed the title feat: implement dialog rails and reasoning traces validator feat: implement validation to forbid dialog rails with reasoning traces Apr 25, 2025
@Pouyanpi Pouyanpi merged commit 46c02bd into develop Apr 25, 2025
71 checks passed
@Pouyanpi Pouyanpi deleted the feat/reasoning-traces-validation branch April 25, 2025 11:58
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