fix(streaming): don't reuse stale across output-rail chunks (#1935)

[Alexander230]

Fix for #1935. Prevents erratic in-place mutations for $bot_message/$user_message .

[greptile-apps]

Greptile Summary

This PR fixes a stale-parameter bug in streaming output rails where action_params (a shared reference from the flow config) was mutated in-place during each streamed chunk, causing subsequent chunks to receive the already-resolved value instead of the $bot_message/$user_message placeholder.

• utils.py: get_action_details_from_flow_id now returns dict(element[\"action_params\"] or {}), shielding the shared flow-config object from any caller that might mutate the returned dict.
• llmrails.py: _prepare_params builds a local resolved_params copy before substituting placeholders, so the original action_params reference (already a copy from utils.py) is never mutated in-place.
• tests/test_streaming_output_rails.py: Two new async tests verify that the substituted text kwarg matches each chunk's bot_message / user_message across multiple streamed chunks.

Confidence Score: 5/5

The change is a well-scoped, targeted fix that copies a shared dict before mutation — no behavioral change for the non-streaming path and no new state is introduced.

Both changed source files apply the fix correctly: utils.py guards the shared flow-config object at the return site, and llmrails.py adds an independent local copy inside _prepare_params. The two new integration tests directly exercise the bug scenario across multiple streamed chunks with both $bot_message and $user_message substitution. No regressions or new issues were found.

No files require special attention.

Important Files Changed

Filename	Overview
nemoguardrails/rails/llm/utils.py	Defensive copy added at the return site of get_action_details_from_flow_id to isolate the shared flow-config action_params from callers that mutate the returned dict.
nemoguardrails/rails/llm/llmrails.py	_prepare_params now creates a local resolved_params copy before performing $bot_message/$user_message substitution, eliminating in-place mutation of the shared action_params reference across streamed chunks.
tests/test_streaming_output_rails.py	Two new async tests exercise the fixed substitution for $bot_message and $user_message across multiple streamed chunks; coverage is appropriate for the bug being fixed.

Sequence Diagram

sequenceDiagram
    participant ChunkLoop as "chunk loop"
    participant GetDetails as "get_action_details_from_flow_id"
    participant FlowConfig as "flow config (shared)"
    participant PrepareParams as "_prepare_params"
    participant Action as "output rail action"

    Note over FlowConfig: "action_params = {text: $bot_message}"

    ChunkLoop->>GetDetails: "get_action_details(flow_id) — chunk N"
    GetDetails->>FlowConfig: "read element[action_params]"
    GetDetails-->>ChunkLoop: "dict(action_params) — fresh copy"

    ChunkLoop->>PrepareParams: "_prepare_params(action_params=copy)"
    PrepareParams->>PrepareParams: "resolved_params = dict(action_params)"
    PrepareParams->>PrepareParams: "resolved_params[text] = bot_chunk_N"
    PrepareParams-->>ChunkLoop: "params with resolved text"

    ChunkLoop->>Action: "execute(params)"
    Action-->>ChunkLoop: "result"

    Note over FlowConfig: "action_params still {text: $bot_message}"

    ChunkLoop->>GetDetails: "get_action_details(flow_id) — chunk N+1"
    GetDetails->>FlowConfig: "read element[action_params] (unchanged)"
    GetDetails-->>ChunkLoop: "dict(action_params) — fresh copy again"

Loading

_{Reviews (2): Last reviewed commit: "fix: test coverage for changes in PR" | Re-trigger Greptile}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ec9085aa-31e9-4868-acf5-fc8d51785e7b

📥 Commits

Reviewing files that changed from the base of the PR and between 64660a9 and e83929d.

📒 Files selected for processing (3)

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

---

📝 Walkthrough

Walkthrough

This PR fixes a bug where streaming output rails were reusing stale parameter substitutions across chunks. The fix copies action parameters before resolving placeholders, ensuring each chunk's $bot_message and $user_message substitutions use current values. A regression test validates the fix.

Changes

Prevent stale parameter substitution in streaming output rails

Layer / File(s)	Summary
Parameter copying for streaming placeholders nemoguardrails/rails/llm/utils.py, nemoguardrails/rails/llm/llmrails.py	get_action_details_from_flow_id now returns a copied action_params dict. In _run_output_rails_in_streaming, placeholder resolution creates a resolved_params copy and mutates only that copy instead of the shared action_params, then passes the resolved copy to output-rail execution.
Streaming parameter substitution test tests/test_streaming_output_rails.py	New test test_streaming_output_rails_no_stale_substituted_param verifies that each streamed chunk's placeholder substitution uses the current bot_message rather than a stale value from an earlier chunk.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• bug: streaming output rails reuse stale $bot_message / $user_message action params across chunks #1935: Related fix addressing the same stale parameter substitution bug by copying and normalizing action_params before resolving placeholders.

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% 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 clearly addresses the main issue: preventing reuse of stale parameters across streaming output-rail chunks, which aligns with the code changes that copy action_params to avoid mutations.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Test Results For Major Changes	✅ Passed	Bug fix (not major feature) with minimal code changes (+11 lines). No performance/numerics implications. Includes comprehensive test validating the fix.

_{✏️ 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.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[ppcvote]

Heads-up for @Alexander230 and @Pouyanpi — the lone Python 3.10 CI failure on this PR isn't from the streaming fix. The failing test is tests/test_parallel_rails.py::test_parallel_rails_success:

FAILED tests/test_parallel_rails.py::test_parallel_rails_success
- AssertionError: Rails processing took too long, parallelization seems to be not working.

Source link — the test compares result.log.stats.input_rails_duration against a wall-clock threshold (< 1.5s). Two of the rails each await asyncio.sleep(1.0), so parallel ~= 1.0s and serial ~= 2.0s; the 1.5s cutoff was meant to split the two. On a slow runner (Ubuntu 3.10 hit it on this PR's 5c423d5, but I'd expect it to spike elsewhere too), parallel duration can exceed 1.5s purely from event-loop scheduling overhead and the test fails despite parallelism working correctly.

A gh search issues + gh search prs against the test name turns up nothing, so it doesn't look like anyone's filed it as a flake yet. I'll open a separate issue + a small PR replacing the wall-clock assertion with a structural overlap check (assert second_rail.started_at < first_rail.finished_at), which proves parallelism without timing dependence. Linking back here when those land so you can pull the re-run once that PR merges.

Independent of all that: nice fix on the streaming bug, especially the test in tests/test_streaming_output_rails.py.

[Pouyanpi]

Thank you @Alexander230 for fixing this bug 🚀