Skip to content

Commit bc61eb8

Browse files
committed
fix(llm): address stack-3 review feedback
- Reorder: store reasoning traces before logging completion - Fix missing underscore in test function names - Fix docstring param name rails -> rail_types - Fall back to empty string for None content in chatmessage conversion
1 parent b6cc892 commit bc61eb8

4 files changed

Lines changed: 12 additions & 12 deletions

File tree

nemoguardrails/actions/llm/utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ async def llm_call(
8282
except Exception as e:
8383
_raise_llm_call_exception(e, model)
8484

85+
_store_reasoning_traces(response)
8586
_log_completion(response)
8687
_update_token_stats(response)
87-
_store_reasoning_traces(response)
8888
_store_tool_calls(response)
8989
_store_response_metadata(response)
9090
return response

nemoguardrails/integrations/langchain/message_utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ def chatmessage_to_langchain_message(msg: ChatMessage) -> BaseMessage:
308308
if cls is ToolMessage:
309309
kwargs["tool_call_id"] = msg.tool_call_id or ""
310310

311-
return cls(content=msg.content, **kwargs)
311+
return cls(content=msg.content or "", **kwargs)
312312

313313

314314
def chatmessages_to_langchain_messages(msgs: List[ChatMessage]) -> List[BaseMessage]:

nemoguardrails/rails/llm/llmrails.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,7 +1456,7 @@ async def check_async(
14561456
messages: List of message dicts with 'role' and 'content' fields.
14571457
Messages can contain any roles, but only user/assistant roles
14581458
determine which rails execute when ``rail_types`` is not provided.
1459-
rails: Optional list of rail types to run, e.g.
1459+
rail_types: Optional list of rail types to run, e.g.
14601460
``[RailType.INPUT]`` or ``[RailType.OUTPUT]``.
14611461
When provided, overrides automatic detection.
14621462
@@ -1525,7 +1525,7 @@ def check(
15251525
15261526
Args:
15271527
messages: List of message dicts with 'role' and 'content' fields.
1528-
rails: Optional list of rail types to run. See check_async() for details.
1528+
rail_types: Optional list of rail types to run. See check_async() for details.
15291529
15301530
Returns:
15311531
RailsResult containing status, content, and optional blocking rail name.

tests/test_tool_calling_utils.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ def test_get_and_clear_tool_calls_contextvar_empty():
5050
assert result is None
5151

5252

53-
def testdicts_to_messages_user():
53+
def test_dicts_to_messages_user():
5454
messages = [{"role": "user", "content": "Hello"}]
5555

5656
result = dicts_to_messages(messages)
@@ -60,7 +60,7 @@ def testdicts_to_messages_user():
6060
assert result[0].content == "Hello"
6161

6262

63-
def testdicts_to_messages_assistant():
63+
def test_dicts_to_messages_assistant():
6464
messages = [{"role": "assistant", "content": "Hi there"}]
6565

6666
result = dicts_to_messages(messages)
@@ -70,7 +70,7 @@ def testdicts_to_messages_assistant():
7070
assert result[0].content == "Hi there"
7171

7272

73-
def testdicts_to_messages_bot():
73+
def test_dicts_to_messages_bot():
7474
messages = [{"type": "bot", "content": "Hello from bot"}]
7575

7676
result = dicts_to_messages(messages)
@@ -80,7 +80,7 @@ def testdicts_to_messages_bot():
8080
assert result[0].content == "Hello from bot"
8181

8282

83-
def testdicts_to_messages_system():
83+
def test_dicts_to_messages_system():
8484
messages = [{"role": "system", "content": "You are a helpful assistant"}]
8585

8686
result = dicts_to_messages(messages)
@@ -90,7 +90,7 @@ def testdicts_to_messages_system():
9090
assert result[0].content == "You are a helpful assistant"
9191

9292

93-
def testdicts_to_messages_tool():
93+
def test_dicts_to_messages_tool():
9494
messages = [{"role": "tool", "content": "Tool result", "tool_call_id": "call_123"}]
9595

9696
result = dicts_to_messages(messages)
@@ -101,7 +101,7 @@ def testdicts_to_messages_tool():
101101
assert result[0].tool_call_id == "call_123"
102102

103103

104-
def testdicts_to_messages_tool_no_id():
104+
def test_dicts_to_messages_tool_no_id():
105105
messages = [{"role": "tool", "content": "Tool result"}]
106106

107107
result = dicts_to_messages(messages)
@@ -112,7 +112,7 @@ def testdicts_to_messages_tool_no_id():
112112
assert result[0].tool_call_id == ""
113113

114114

115-
def testdicts_to_messages_mixed():
115+
def test_dicts_to_messages_mixed():
116116
messages = [
117117
{"role": "system", "content": "System prompt"},
118118
{"role": "user", "content": "User message"},
@@ -130,7 +130,7 @@ def testdicts_to_messages_mixed():
130130
assert result[3].tool_call_id == "call_456"
131131

132132

133-
def testdicts_to_messages_unknown_type():
133+
def test_dicts_to_messages_unknown_type():
134134
messages = [{"role": "unknown", "content": "Unknown message"}]
135135

136136
with pytest.raises(ValueError, match="Unknown message type: unknown"):

0 commit comments

Comments
 (0)