fix: prevent stale action params in streaming output rails across chunks

[nac7]

Summary

Fixes #1935.

When streaming output rails invoke an action with explicit colang params (e.g. execute capture_output(text=$bot_message)), the $bot_message placeholder is resolved inside _prepare_params(). The bug: get_action_details_from_flow_id() returns the live dict from the parsed flow config — not a copy — so _prepare_params() was mutating it in place. After the first chunk, action_params["text"] permanently stored the first chunk's text instead of the "$bot_message" sentinel. Every subsequent chunk received the stale first-chunk value.

Root causes fixed:

• _prepare_params() mutated action_params in place (the live flow-config dict)
• Mutable default argument action_params: Dict[str, Any] = {} (secondary issue)

Fix: shallow-copy action_params at the start of _prepare_params() before any substitution, so the original parsed-flow dict is never touched.

Affected rails (any that pass $bot_message or $user_message as explicit action params): prompt_security, privateai, gliner, policyai, trend_micro, activefence, ai_defense, regex.

Changes

• nemoguardrails/rails/llm/llmrails.py: add action_params = dict(action_params or {}) shallow copy + fix mutable default arg
• tests/test_streaming_output_rails.py: 3 new regression tests
  • test_streaming_action_params_not_stale_across_chunks — verifies each chunk batch gets the correct substituted value (not stale)
  • test_streaming_action_params_original_flow_config_not_mutated — verifies the parsed flow config dict is not mutated after streaming
  • test_streaming_user_message_param_substituted — covers the $user_message substitution branch

Test plan

• All 13 tests in tests/test_streaming_output_rails.py pass
• ruff check passes on both changed files
• CI green across Python versions

Summary by CodeRabbit

• Bug Fixes

  • Resolved an issue where streaming output rails would reuse stale parameter values across chunks, ensuring parameters are correctly substituted for each streamed chunk.

• Tests

  • Added regression tests for streaming output rails parameter handling.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR fixes a parameter mutation bug in streaming output rails where placeholder-substituted values ($bot_message, $user_message) were reused across streamed chunks. The fix shallow-copies action_params before substitution and adds comprehensive regression test coverage validating the fix across multiple placeholder scenarios.

Changes

Streaming output rails parameter mutation fix

Layer / File(s)	Summary
Parameter copy to prevent mutation nemoguardrails/rails/llm/llmrails.py	_prepare_params now accepts action_params as an optional dict (default None) instead of a mutable default, and shallow-copies action_params before substituting placeholders like $bot_message and $user_message. This prevents the parsed flow configuration's original action_params dict from being permanently mutated by the first processed chunk.
Regression test coverage for placeholder substitution tests/test_streaming_output_rails.py	Three new async regression tests verify the fix: (1) $bot_message action params receive fresh values across multiple streamed chunks instead of reusing the first chunk's stale value; (2) the original parsed flow configuration's action_params dict retains placeholder strings like "$bot_message" after streaming (is not mutated); and (3) $user_message placeholder substitution is correctly applied with the current user message content on each invocation.

---

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: prevent stale action params in streaming output rails across chunks' directly reflects the main change: preventing stale action parameter values from persisting across streamed chunks.
Linked Issues check	✅ Passed	The PR fully addresses issue #1935 by implementing the proposed fix: shallow-copying action_params in _prepare_params() and removing the mutable default argument, plus adding comprehensive regression tests.
Out of Scope Changes check	✅ Passed	All changes are directly in scope: the fix targets the specific streaming output rails mutation bug identified in #1935, and tests validate the solution without introducing unrelated modifications.
Test Results For Major Changes	✅ Passed	Minor bug fix with comprehensive test documentation. PR includes test results (13 tests pass), code quality checks (ruff), and CI verification. Not a major feature or performance-critical change.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_streaming_output_rails.py (1)

623-625: 💤 Low value

Optional: Add parentheses for operator precedence clarity.

The assertion logic is correct, but the mixed and/or precedence is flagged by static analysis. Adding explicit parentheses improves readability without changing behavior.

♻️ Proposed refactor for clarity

-    assert isinstance(received, dict) and received.get("content") == "hello there" or received == "hello there", (
+    assert (isinstance(received, dict) and received.get("content") == "hello there") or received == "hello there", (
         f"$user_message was not substituted correctly: {received!r}"
     )

🤖 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 `@tests/test_streaming_output_rails.py` around lines 623 - 625, The assertion
mixes and/or which can be ambiguous to linters; wrap the boolean subexpressions
in parentheses to make operator precedence explicit—specifically modify the
assert that checks the variable received (the expression beginning with assert
isinstance(received, dict)...) so the dict-check and content-check are grouped
together and the alternative equality check (received == "hello there") is
grouped, preserving the same logic but clarifying precedence for static
analysis.

Source: Linters/SAST tools

🤖 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 `@tests/test_streaming_output_rails.py`:
- Around line 623-625: The assertion mixes and/or which can be ambiguous to
linters; wrap the boolean subexpressions in parentheses to make operator
precedence explicit—specifically modify the assert that checks the variable
received (the expression beginning with assert isinstance(received, dict)...) so
the dict-check and content-check are grouped together and the alternative
equality check (received == "hello there") is grouped, preserving the same logic
but clarifying precedence for static analysis.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7beaebd7-29c6-4191-a29e-548ba09d4572

📥 Commits

Reviewing files that changed from the base of the PR and between 1839dd2 and 157c893.

📒 Files selected for processing (2)

• nemoguardrails/rails/llm/llmrails.py
• tests/test_streaming_output_rails.py

[greptile-apps]

Greptile Summary

This PR fixes a stale-parameter bug in streaming output rails where _prepare_params() was mutating the live flow-config dict returned by get_action_details_from_flow_id(), causing every chunk after the first to receive the first chunk's resolved text instead of re-substituting the $bot_message placeholder.

• Core fix (llmrails.py): adds action_params = dict(action_params or {}) at the top of _prepare_params to shallow-copy before substitution, and changes the mutable default argument = {} to = None. A shallow copy is sufficient because all substituted values ($bot_message, $user_message) are immutable strings.
• Tests (test_streaming_output_rails.py): three new regression tests verify correct per-chunk substitution, flow-config immutability after streaming, and the $user_message substitution branch; all include return_exceptions=True on the teardown asyncio.gather.
• Unrelated CI change (_test.yml): codecov/codecov-action is downgraded from v5 to v4 without explanation in the PR description.

Confidence Score: 5/5

Safe to merge — the two-line change is minimal, precisely targeted, and well-covered by the new regression tests.

The fix is a shallow copy of a dict whose values are immutable strings, so the copy depth is correct. The mutable default argument is also corrected. Three new tests directly exercise the changed code path, including the no-mutation invariant on the live flow-config dict. The only other change is an unexplained CI version downgrade that has no effect on correctness.

.github/workflows/_test.yml — the unexplained codecov-action downgrade from v5 to v4 should be confirmed intentional.

Important Files Changed

Filename	Overview
nemoguardrails/rails/llm/llmrails.py	Fixes mutable default argument and in-place mutation of the live flow-config dict in _prepare_params; shallow copy is sufficient since all substituted values are immutable strings.
tests/test_streaming_output_rails.py	Adds three targeted regression tests covering stale-params across chunks, flow-config immutability, and $user_message substitution; all include return_exceptions=True on the teardown gather.
.github/workflows/_test.yml	Downgrades codecov/codecov-action from v5 to v4 with no explanation in the PR description — unrelated to the bug fix and may be accidental.

Sequence Diagram

sequenceDiagram
    participant SH as StreamingHandler
    participant SA as stream_async
    participant PP as _prepare_params
    participant FC as flow config dict<br/>(action_params)
    participant AC as registered action

    loop each chunk batch
        SH->>SA: chunk_batch ("alpha ", "beta", …)
        SA->>SA: "get_action_details_from_flow_id()<br/>returns live FC reference"
        SA->>PP: "flow_id, action_name,<br/>bot_response_chunk,<br/>action_params (FC ref)"
        PP->>PP: "action_params = dict(action_params or {})<br/>← shallow copy (NEW)"
        PP->>PP: "substitute $bot_message → chunk text<br/>on the COPY, not FC"
        note over FC: FC["chunk"] stays "$bot_message" ✓
        PP-->>SA: resolved params dict
        SA->>AC: "call action(chunk="alpha ")<br/>call action(chunk="beta")"
    end

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/workflows/_test.yml:107
**Unexplained downgrade of `codecov-action`**

This PR downgrades `codecov/codecov-action` from `v5` to `v4`, but the change isn't mentioned anywhere in the PR description or commit message. This is unrelated to the stale-action-params fix. If this was intentional (e.g., to work around a v5 regression), a brief comment explaining why would help reviewers — and if it's accidental, it should be reverted to avoid silently rolling back any v5 improvements.

_{Reviews (2): Last reviewed commit: "fix: address review comments and resolve..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[nac7]

Hi @Pouyanpi , if you have some time, could you please help with this PR review? Thanks!