Skip to content

Commit 9a70fca

Browse files
authored
fix(config): correct dialog rails activation logic (#1161)
* 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. * Update tests/test_config_validation.py based on reveiw
1 parent 2dce5a0 commit 9a70fca

File tree

2 files changed

+35
-56
lines changed

2 files changed

+35
-56
lines changed

nemoguardrails/rails/llm/config.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,11 +1203,10 @@ def check_reasoning_traces_with_dialog_rails(cls, values):
12031203
]
12041204

12051205
# dialog rails are activated (explicitly or implicitly)
1206-
has_dialog_rails = (
1207-
bool(dialog_rails)
1208-
or bool(values.get("user_messages"))
1209-
or bool(values.get("bot_messages"))
1210-
or bool(values.get("flows"))
1206+
has_dialog_rails = bool(dialog_rails) or (
1207+
bool(values.get("user_messages"))
1208+
and bool(values.get("bot_messages"))
1209+
# and bool(values.get("flows"))
12111210
)
12121211

12131212
if has_dialog_rails:

tests/test_config_validation.py

Lines changed: 31 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,10 @@ def test_reasoning_traces_with_implicit_dialog_rails_flows_only():
295295
define flow
296296
user express greeting
297297
bot express greeting
298+
define user express greeting
299+
"hi"
300+
define bot express greeting
301+
"HI HI"
298302
""",
299303
)
300304

@@ -311,65 +315,41 @@ def test_reasoning_traces_with_implicit_dialog_rails_flows_only():
311315

312316

313317
def test_reasoning_traces_with_implicit_dialog_rails_user_messages_only():
314-
"""Test that reasoning traces cannot be enabled when dialog rails are implicitly enabled thru user messages only."""
318+
"""Test that reasoning traces cannot be enabled when dialog rails are implicitly enabled through user messages (user canonical forms) only."""
315319

316-
with pytest.raises(ValueError) as exc_info:
317-
_ = RailsConfig.from_content(
318-
yaml_content="""
319-
models:
320-
- type: main
321-
engine: openai
322-
model: gpt-3.5-turbo-instruct
323-
reasoning_config:
324-
remove_thinking_traces: false
325-
""",
326-
colang_content="""
320+
_ = RailsConfig.from_content(
321+
yaml_content="""
322+
models:
323+
- type: main
324+
engine: openai
325+
model: gpt-3.5-turbo-instruct
326+
reasoning_config:
327+
remove_thinking_traces: false
328+
""",
329+
colang_content="""
327330
define user express greeting
328331
"hello"
329332
"hi"
330-
""",
331-
)
332-
333-
assert "Main model has reasoning traces enabled in config.yml" in str(
334-
exc_info.value
335-
)
336-
assert "Reasoning traces must be disabled when dialog rails are present" in str(
337-
exc_info.value
338-
)
339-
assert (
340-
"Please update your config.yml to set 'remove_thinking_traces: true' under reasoning_config"
341-
in str(exc_info.value)
333+
""",
342334
)
343335

344336

345-
def test_reasoning_traces_with_implicit_dialog_rails_bot_messages_only():
346-
"""Test that reasoning traces cannot be enabled when dialog rails are implicitly enabled thru bot messages only."""
337+
def test_reasoning_traces_with_bot_messages_only():
338+
"""Test that reasoning traces can exist with bot messages only."""
347339

348-
with pytest.raises(ValueError) as exc_info:
349-
_ = RailsConfig.from_content(
350-
yaml_content="""
351-
models:
352-
- type: main
353-
engine: openai
354-
model: gpt-3.5-turbo-instruct
355-
reasoning_config:
356-
remove_thinking_traces: false
357-
""",
358-
colang_content="""
359-
define bot express greeting
360-
"Hello there!"
361-
""",
362-
)
363-
364-
assert "Main model has reasoning traces enabled in config.yml" in str(
365-
exc_info.value
366-
)
367-
assert "Reasoning traces must be disabled when dialog rails are present" in str(
368-
exc_info.value
369-
)
370-
assert (
371-
"Please update your config.yml to set 'remove_thinking_traces: true' under reasoning_config"
372-
in str(exc_info.value)
340+
_ = RailsConfig.from_content(
341+
yaml_content="""
342+
models:
343+
- type: main
344+
engine: openai
345+
model: gpt-3.5-turbo-instruct
346+
reasoning_config:
347+
remove_thinking_traces: False
348+
""",
349+
colang_content="""
350+
define bot express greeting
351+
"Hello there!"
352+
""",
373353
)
374354

375355

0 commit comments

Comments
 (0)