fix(streaming): pass user content to output rails#2081
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe helper used by output-rails streaming now returns the latest user message content string, or an empty string when unavailable. A new async streaming test verifies ChangesStreaming user content propagation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nemoguardrails/rails/llm/llmrails.py (1)
1814-1820: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHelper now correctly returns the user message content string (or
""), matching the downstream string contract forcontext["user_message"]and$user_messageresolution.Optional: the
-> Anyannotation is now imprecise since this always returns a string. Tightening it to-> strbetter documents the contract for callers like_prepare_params/_prepare_context_for_parallel_rails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nemoguardrails/rails/llm/llmrails.py` around lines 1814 - 1820, The helper that extracts the latest user message currently has an overly broad return annotation, even though it always returns a string or an empty string. Update the return type on the user-message helper in LLMRails to str so it matches the actual contract used by _prepare_params and _prepare_context_for_parallel_rails, while keeping the current string-return behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@nemoguardrails/rails/llm/llmrails.py`:
- Around line 1814-1820: The helper that extracts the latest user message
currently has an overly broad return annotation, even though it always returns a
string or an empty string. Update the return type on the user-message helper in
LLMRails to str so it matches the actual contract used by _prepare_params and
_prepare_context_for_parallel_rails, while keeping the current string-return
behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: eeafdb04-e4bc-42da-ac54-56d58a23bff2
📒 Files selected for processing (2)
nemoguardrails/rails/llm/llmrails.pytests/test_streaming_output_rails.py
69745f6 to
81ef48f
Compare
Description
pass user content to output rails, fixes a bug that discovered in #1978 , addresses #1978 (comment).
Root cause
streaming output rails received the wrong shape for
context["user_message"]. Thenested helper
_get_latest_user_message()returned the whole message dict, so forstream_async(messages=[...])calls the output-rail context (and the$user_messageaction param) got
{"role": "user", "content": "hello"}instead of"hello". Thatmalformed value reaches runtime execution: both
content safetyandself check outputtemplate
user_messageinto their provider prompts, and custom actions readingcontext["user_message"]get a dict. Rails whose decision depends only onbot_messagestill behaved correctly, which is why it stayed latent.
Fix: return
message["content"](or""when there is no user message).scope: streaming output rails with
messages=[...]only.stream_async(prompt="...")short-circuits to the string and was unaffected; non-streaming rails resolve
$user_messagevia the Colang runtime, a different path.Related Issue(s)
AI Assistance
Checklist
Summary by CodeRabbit
Bug Fixes
Tests