Skip to content

Commit 1fd71cc

Browse files
committed
fix(llmrails): move LLM isolation setup to after KB initialization
The LLM isolation code was called too early in __init__, before all components were fully initialized. This caused flow matching to fail when trying to resolve rail flow IDs. Move _create_isolated_llms_for_actions() call to after KB setup to ensure all initialization is complete before creating isolated LLMs.
1 parent d1b35c8 commit 1fd71cc

4 files changed

Lines changed: 90 additions & 88 deletions

File tree

nemoguardrails/rails/llm/llmrails.py

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,8 @@ def __init__(
284284
# We also register the kb as a parameter that can be passed to actions.
285285
self.runtime.register_action_param("kb", self.kb)
286286

287+
# detect actions that need isolated LLM instances and create them
288+
self._create_isolated_llms_for_actions()
287289
# Reference to the general ExplainInfo object.
288290
self.explain_info = None
289291

@@ -507,9 +509,6 @@ def _init_llms(self):
507509

508510
self.runtime.register_action_param("llms", llms)
509511

510-
# detect actions that need isolated LLM instances and create them
511-
self._create_isolated_llms_for_actions()
512-
513512
def _create_isolated_llms_for_actions(self):
514513
"""Create isolated LLM copies for all actions that accept 'llm' parameter."""
515514
if not self.llm:
@@ -525,17 +524,38 @@ def _create_isolated_llms_for_actions(self):
525524
)
526525

527526
created_count = 0
528-
# Get the actions from flows defined in rails config
529-
get_action_details = partial(
530-
get_action_details_from_flow_id, flows=self.config.flows
531-
)
527+
532528
configured_actions_names = []
533-
for flow_id in self.config.rails.input.flows:
534-
action_name, _ = get_action_details(flow_id)
535-
configured_actions_names.append(action_name)
536-
for flow_id in self.config.rails.output.flows:
537-
action_name, _ = get_action_details(flow_id)
538-
configured_actions_names.append(action_name)
529+
try:
530+
if self.config.flows:
531+
get_action_details = partial(
532+
get_action_details_from_flow_id, flows=self.config.flows
533+
)
534+
for flow_id in self.config.rails.input.flows:
535+
action_name, _ = get_action_details(flow_id)
536+
configured_actions_names.append(action_name)
537+
for flow_id in self.config.rails.output.flows:
538+
action_name, _ = get_action_details(flow_id)
539+
configured_actions_names.append(action_name)
540+
else:
541+
# for configurations without flow definitions, use all actions that need LLMs
542+
print(
543+
"No flow definitions found, creating isolated LLMs for all actions requiring them"
544+
)
545+
configured_actions_names = list(actions_needing_llms)
546+
except Exception as e:
547+
# if flow matching fails, fall back to all actions that need LLMs
548+
log.info(
549+
"No flow definitions found, creating isolated LLMs for all actions requiring them"
550+
)
551+
configured_actions_names = list(actions_needing_llms)
552+
except Exception as e:
553+
# if flow matching fails, fall back to all actions that need LLMs
554+
log.warning(
555+
"Flow matching failed (%s), creating isolated LLMs for all actions requiring them",
556+
e,
557+
)
558+
configured_actions_names = list(actions_needing_llms)
539559

540560
for action_name in configured_actions_names:
541561
if action_name not in actions_needing_llms:

nemoguardrails/rails/llm/utils.py

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
# See the License for the specific language governing permissions and
1414
# limitations under the License.
1515
import json
16-
from typing import Any, Dict, List, Optional, Tuple, Union
16+
from typing import Any, Dict, List, Tuple, Union
17+
18+
from nemoguardrails.colang.v1_0.runtime.flows import _normalize_flow_id
1719

1820

1921
def get_history_cache_key(messages: List[dict]) -> str:
@@ -61,37 +63,24 @@ def get_history_cache_key(messages: List[dict]) -> str:
6163
def get_action_details_from_flow_id(
6264
flow_id: str,
6365
flows: List[Union[Dict, Any]],
64-
prefixes: Optional[List[str]] = None,
6566
) -> Tuple[str, Any]:
6667
"""Get the action name and parameters from the flow id.
6768
6869
First, try to find an exact match.
6970
If not found, then if the provided flow_id starts with one of the special prefixes,
7071
return the first flow whose id starts with that same prefix.
7172
"""
72-
supported_prefixes = [
73-
"content safety check output",
74-
"topic safety check output",
75-
]
76-
if prefixes:
77-
supported_prefixes.extend(prefixes)
7873

7974
candidate_flow = None
8075

8176
for flow in flows:
8277
# If exact match, use it
83-
if flow["id"] == flow_id:
78+
flow_id = _normalize_flow_id(flow_id)
79+
normalized_flow_id = _normalize_flow_id(flow_id)
80+
for flow in flows:
81+
# If exact match, use it
82+
if flow["id"] == normalized_flow_id:
8483
candidate_flow = flow
85-
break
86-
87-
# If no exact match, check if both the provided flow_id and this flow's id share a special prefix
88-
for prefix in supported_prefixes:
89-
if flow_id.startswith(prefix) and flow["id"].startswith(prefix):
90-
candidate_flow = flow
91-
# We don't break immediately here because an exact match would have been preferred,
92-
# but since we're in the else branch it's fine to choose the first matching candidate.
93-
# TODO:we should avoid having multiple matchin prefixes
94-
break
9584

9685
if candidate_flow is not None:
9786
break

tests/test_llm_isolation.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,3 +536,44 @@ def test_create_isolated_llms_handles_empty_rails_config(self, rails_with_mock_l
536536
mock_get_action.assert_not_called()
537537

538538
rails.runtime.register_action_param.assert_not_called()
539+
540+
def test_llm_isolation_timing_with_empty_flows(self, rails_with_mock_llm, caplog):
541+
"""Test that LLM isolation handles empty flows gracefully during initialization.
542+
543+
This test reproduces the timing issue where _create_isolated_llms_for_actions()
544+
was called before flows were properly loaded. Before the fix, this would fail
545+
when trying to resolve rail flow IDs against an empty flows list, causing
546+
LLM isolation to fail silently with a warning log.
547+
"""
548+
rails = rails_with_mock_llm
549+
550+
rails.llm = MockLLM(model_kwargs={}, temperature=0.7)
551+
552+
# simulate the problematic scenario: rail flows defined but config.flows empty
553+
rails.config.rails = Mock()
554+
rails.config.rails.input = Mock()
555+
rails.config.rails.output = Mock()
556+
rails.config.rails.input.flows = [
557+
"content safety check input $model=content_safety"
558+
]
559+
rails.config.rails.output.flows = [
560+
"content safety check output $model=content_safety"
561+
]
562+
rails.config.flows = [] # Empty flows list (timing issue scenario)
563+
564+
rails.runtime = Mock()
565+
rails.runtime.action_dispatcher = MockActionDispatcher()
566+
rails.runtime.registered_action_params = {}
567+
rails.runtime.register_action_param = Mock()
568+
569+
# before the fix, this would log a warning about failing to create isolated LLMs
570+
# after the fix, it should handle empty flows gracefully without the warning
571+
rails._create_isolated_llms_for_actions()
572+
573+
warning_messages = [
574+
record.message for record in caplog.records if record.levelname == "WARNING"
575+
]
576+
assert not any(
577+
"Failed to create isolated LLMs for actions" in msg
578+
for msg in warning_messages
579+
), f"Fix failed: Warning still logged: {warning_messages}"

tests/test_rails_llm_utils.py

Lines changed: 8 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,11 @@ def test_get_action_details_from_flow_id_exact_match():
145145
assert action_params == {"param1": "value1"}
146146

147147

148-
def test_get_action_details_from_flow_id_prefix_match():
149-
"""Test get_action_details_from_flow_id with prefix matching."""
148+
def test_get_action_details_from_flow_id_content_safety():
149+
"""Test get_action_details_from_flow_id ."""
150150
flows = [
151151
{
152-
"id": "content safety check output with model gpt-4",
152+
"id": "content safety check output",
153153
"elements": [
154154
{
155155
"_type": "run_action",
@@ -165,17 +165,17 @@ def test_get_action_details_from_flow_id_prefix_match():
165165
]
166166

167167
action_name, action_params = get_action_details_from_flow_id(
168-
"content safety check output", flows
168+
"content safety check output $model=anothe_model_config", flows
169169
)
170170
assert action_name == "content_safety_check"
171171
assert action_params == {"model": "gpt-4"}
172172

173173

174-
def test_get_action_details_from_flow_id_topic_safety_prefix():
175-
"""Test get_action_details_from_flow_id with topic safety prefix."""
174+
def test_get_action_details_from_flow_id_topic_safety():
175+
"""Test get_action_details_from_flow_id with topic safety."""
176176
flows = [
177177
{
178-
"id": "topic safety check output with model claude",
178+
"id": "topic safety check output",
179179
"elements": [
180180
{
181181
"_type": "run_action",
@@ -191,38 +191,12 @@ def test_get_action_details_from_flow_id_topic_safety_prefix():
191191
]
192192

193193
action_name, action_params = get_action_details_from_flow_id(
194-
"topic safety check output", flows
194+
"topic safety check output $model=claude_model", flows
195195
)
196196
assert action_name == "topic_safety_check"
197197
assert action_params == {"model": "claude"}
198198

199199

200-
def test_get_action_details_from_flow_id_custom_prefixes():
201-
"""Test get_action_details_from_flow_id with custom prefixes."""
202-
flows = [
203-
{
204-
"id": "custom prefix flow with params",
205-
"elements": [
206-
{
207-
"_type": "run_action",
208-
"_source_mapping": {
209-
"filename": "custom.co",
210-
"line_text": "execute custom_action",
211-
},
212-
"action_name": "custom_action",
213-
"action_params": {"custom": "value"},
214-
}
215-
],
216-
}
217-
]
218-
219-
action_name, action_params = get_action_details_from_flow_id(
220-
"custom prefix flow", flows, prefixes=["custom prefix"]
221-
)
222-
assert action_name == "custom_action"
223-
assert action_params == {"custom": "value"}
224-
225-
226200
def test_get_action_details_from_flow_id_no_match():
227201
"""Test get_action_details_from_flow_id when no flow matches."""
228202
flows = [
@@ -410,28 +384,6 @@ def test_get_action_details_exact_match_not_colang_2(dummy_flows):
410384
assert "No run_action element found for flow_id" in str(exc_info.value)
411385

412386

413-
def test_get_action_details_prefix_match(dummy_flows):
414-
# For a flow_id that starts with the prefix "other_flow",
415-
# we expect to retrieve the action details from the flow whose id starts with that prefix.
416-
# we expect a result since we are passing the prefixes argument.
417-
action_name, action_params = get_action_details_from_flow_id(
418-
"other_flow", dummy_flows, prefixes=["other_flow"]
419-
)
420-
assert action_name == "other_action"
421-
assert action_params == {"param2": "value2"}
422-
423-
424-
def test_get_action_details_prefix_match_unsupported_prefix(dummy_flows):
425-
# For a flow_id that starts with the prefix "other_flow",
426-
# we expect to retrieve the action details from the flow whose id starts with that prefix.
427-
# but as the prefix is not supported, we expect a ValueError.
428-
429-
with pytest.raises(ValueError) as exc_info:
430-
get_action_details_from_flow_id("other_flow", dummy_flows)
431-
432-
assert "No action found for flow_id" in str(exc_info.value)
433-
434-
435387
def test_get_action_details_no_match(dummy_flows):
436388
# Tests that a non matching flow_id raises a ValueError
437389
with pytest.raises(ValueError) as exc_info:

0 commit comments

Comments
 (0)