From 6b174d90f45d161c78c41e365826d2f8867cbfc2 Mon Sep 17 00:00:00 2001 From: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com> Date: Tue, 29 Apr 2025 21:11:23 +0200 Subject: [PATCH 1/2] fix(config): correct dialog rails activation logic Fixed the `has_dialog_rails` logic in `RailsConfig` to require both `user_messages` and `bot_messages` for implicit activation. Removed `flows` from the condition to align with expected behavior. test(config): adjust tests for updated dialog rails logic Modified test cases to reflect the corrected dialog rails activation logic. --- nemoguardrails/rails/llm/config.py | 9 ++-- tests/test_config_validation.py | 82 +++++++++++------------------- 2 files changed, 35 insertions(+), 56 deletions(-) diff --git a/nemoguardrails/rails/llm/config.py b/nemoguardrails/rails/llm/config.py index 532fb3b96..2e4f36290 100644 --- a/nemoguardrails/rails/llm/config.py +++ b/nemoguardrails/rails/llm/config.py @@ -1199,11 +1199,10 @@ def check_reasoning_traces_with_dialog_rails(cls, values): ] # dialog rails are activated (explicitly or implicitly) - has_dialog_rails = ( - bool(dialog_rails) - or bool(values.get("user_messages")) - or bool(values.get("bot_messages")) - or bool(values.get("flows")) + has_dialog_rails = bool(dialog_rails) or ( + bool(values.get("user_messages")) + and bool(values.get("bot_messages")) + # and bool(values.get("flows")) ) if has_dialog_rails: diff --git a/tests/test_config_validation.py b/tests/test_config_validation.py index 8b6040a30..b96c292d9 100644 --- a/tests/test_config_validation.py +++ b/tests/test_config_validation.py @@ -289,12 +289,16 @@ def test_reasoning_traces_with_implicit_dialog_rails_flows_only(): engine: openai model: gpt-3.5-turbo-instruct reasoning_config: - remove_thinking_traces: false + remove_thinking_traces: False """, colang_content=""" define flow user express greeting bot express greeting + define user express greeting + "hi" + define bot express greeting + "HI HI" """, ) @@ -313,63 +317,39 @@ def test_reasoning_traces_with_implicit_dialog_rails_flows_only(): def test_reasoning_traces_with_implicit_dialog_rails_user_messages_only(): """Test that reasoning traces cannot be enabled when dialog rails are implicitly enabled thru user messages only.""" - with pytest.raises(ValueError) as exc_info: - _ = RailsConfig.from_content( - yaml_content=""" - models: - - type: main - engine: openai - model: gpt-3.5-turbo-instruct - reasoning_config: - remove_thinking_traces: false - """, - colang_content=""" + _ = RailsConfig.from_content( + yaml_content=""" + models: + - type: main + engine: openai + model: gpt-3.5-turbo-instruct + reasoning_config: + remove_thinking_traces: False + """, + colang_content=""" define user express greeting "hello" "hi" - """, - ) - - assert "Main model has reasoning traces enabled in config.yml" in str( - exc_info.value - ) - assert "Reasoning traces must be disabled when dialog rails are present" in str( - exc_info.value - ) - assert ( - "Please update your config.yml to set 'remove_thinking_traces: true' under reasoning_config" - in str(exc_info.value) + """, ) -def test_reasoning_traces_with_implicit_dialog_rails_bot_messages_only(): - """Test that reasoning traces cannot be enabled when dialog rails are implicitly enabled thru bot messages only.""" +def test_reasoning_traces_with_bot_messages_only(): + """Test that reasoning taces can exist with bot messages only.""" - with pytest.raises(ValueError) as exc_info: - _ = RailsConfig.from_content( - yaml_content=""" - models: - - type: main - engine: openai - model: gpt-3.5-turbo-instruct - reasoning_config: - remove_thinking_traces: false - """, - colang_content=""" - define bot express greeting - "Hello there!" - """, - ) - - assert "Main model has reasoning traces enabled in config.yml" in str( - exc_info.value - ) - assert "Reasoning traces must be disabled when dialog rails are present" in str( - exc_info.value - ) - assert ( - "Please update your config.yml to set 'remove_thinking_traces: true' under reasoning_config" - in str(exc_info.value) + _ = RailsConfig.from_content( + yaml_content=""" + models: + - type: main + engine: openai + model: gpt-3.5-turbo-instruct + reasoning_config: + remove_thinking_traces: False + """, + colang_content=""" + define bot express greeting + "Hello there!" + """, ) From 836008d9264ce97f7f6cbb3e2e8dca662e90d4ec Mon Sep 17 00:00:00 2001 From: Pouyan <13303554+Pouyanpi@users.noreply.github.com> Date: Thu, 1 May 2025 13:10:11 +0200 Subject: [PATCH 2/2] Update tests/test_config_validation.py based on reveiw --- tests/test_config_validation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_config_validation.py b/tests/test_config_validation.py index b96c292d9..145442a52 100644 --- a/tests/test_config_validation.py +++ b/tests/test_config_validation.py @@ -289,7 +289,7 @@ def test_reasoning_traces_with_implicit_dialog_rails_flows_only(): engine: openai model: gpt-3.5-turbo-instruct reasoning_config: - remove_thinking_traces: False + remove_thinking_traces: false """, colang_content=""" define flow @@ -315,7 +315,7 @@ def test_reasoning_traces_with_implicit_dialog_rails_flows_only(): def test_reasoning_traces_with_implicit_dialog_rails_user_messages_only(): - """Test that reasoning traces cannot be enabled when dialog rails are implicitly enabled thru user messages only.""" + """Test that reasoning traces cannot be enabled when dialog rails are implicitly enabled through user messages (user canonical forms) only.""" _ = RailsConfig.from_content( yaml_content=""" @@ -324,7 +324,7 @@ def test_reasoning_traces_with_implicit_dialog_rails_user_messages_only(): engine: openai model: gpt-3.5-turbo-instruct reasoning_config: - remove_thinking_traces: False + remove_thinking_traces: false """, colang_content=""" define user express greeting @@ -335,7 +335,7 @@ def test_reasoning_traces_with_implicit_dialog_rails_user_messages_only(): def test_reasoning_traces_with_bot_messages_only(): - """Test that reasoning taces can exist with bot messages only.""" + """Test that reasoning traces can exist with bot messages only.""" _ = RailsConfig.from_content( yaml_content="""