Skip to content

feat(guardrails): add guardrailed banking agent example (RHAIENG-6262) - #282

Merged
tarun-etikala merged 25 commits into
red-hat-data-services:mainfrom
tarun-etikala:rhaieng-6262-guardrailed-agent
Jul 28, 2026
Merged

tarun-etikala merged 25 commits into
red-hat-data-services:mainfrom
tarun-etikala:rhaieng-6262-guardrailed-agent

Conversation

@tarun-etikala

@tarun-etikala tarun-etikala commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Adds agents/langgraph/examples/guardrailed_agent/ — a banking customer service agent with NeMo Guardrails safety layer
  • Demonstrates content safety (S1–S13), topical guardrails (banking domain), and regex-based input filtering using the proxy pattern (passthrough: true)
  • Adapted from the react_agent template with a banking system prompt and check_account_balance dummy tool
  • Pins nemoguardrails[server]==0.21.0 (matches RHOAI) as an optional dependency
  • 29 structural tests validating guardrails config and packaging

Architecture

User → Agent (8000) → NeMo Guardrails (8090) → Ollama (11434)

Three layered input rails: regex (instant) → content safety (LLM) → topic safety (LLM), plus content safety output rail. passthrough: true makes NeMo act as a pure safety filter without running its conversational engine.

Key decisions

  • passthrough: true — required for proxy pattern; without it NeMo runs 3 extra LLM calls and ignores the agent's system prompt/tools
  • api_key in parameters — api_key_env_var doesn't work with v0.21.0 LangChain initializer
  • Ollama direct (11434) — OGX has a pre-existing model name case mismatch (Llama3.1:8B vs llama3.1:8b) that's a separate repo-wide issue
  • Guardrails block error handling — _is_guardrails_block() helper catches NeMo OpenAIAPIError in both streaming and non-streaming paths, returns clean refusal instead of HTTP 500
  • nemoguardrails as optional dep — agent doesn't import it; NeMo runs as a separate server
  • Health probe — /health endpoint probes the NeMo Guardrails server and reports guardrails_reachable status

Differences from react_agent template

Area Change
main.py Updated imports, FastAPI title, _is_guardrails_block() helper, guardrails health probe, images path fix
src/guardrailed_agent/agent.py Banking system prompt, check_account_balance tool
src/guardrailed_agent/tools.py Banking-themed dummy tool (replaces dummy_web_search)
guardrails/safety/ New: config.yaml, prompts.yml, rails.co
pyproject.toml Removed unused deps (milvus, pymilvus, pypdf, flask, chardet), added guardrails optional extra, added httpx, fixed Python version <3.14
Makefile Removed ogx-server, added guardrails-config (python3+yaml), guardrails-server
.env.example BASE_URL points to NeMo proxy (8090), MODEL_ID=llama3.1:8b

E2E verified

Test Mode Expected Result
On-topic banking question non-stream Response ✅
Toxic input ("build a bomb") non-stream Blocked ✅
Off-topic ("recipe for cake") non-stream Blocked ✅
Greeting non-stream Response ✅
Off-topic (3 attempts) streaming Blocked cleanly ✅
On-topic streaming Response ✅
Health (guardrails up) — 200, reachable: true ✅
Health (guardrails down) — 503, reachable: false ✅

Test plan

  • make init && make env && make ollama
  • make guardrails-server (terminal 1)
  • make run-app (terminal 2)
  • Run curl tests from README step 6
  • make test — 29 structural tests pass
  • Stop guardrails-server, verify /health returns 503

Related

  • Epic: RHAIENG-6251
  • Spike: RHAIENG-5947 (validated all integration patterns)
  • Remaining stories: RHAIENG-6263 (compose), 6264 (RHOAI manifests), 6265 (docs), 6266 (decision guide), 6267 (pytest behavioral)

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a NeMo Guardrails-protected LangGraph banking agent with OpenAI-compatible streaming APIs, a Flask/browser playground, local and OpenShift deployment tooling, container packaging, MLflow tracing, guardrail configuration, and structural tests.

Changes

Guardrailed agent example

Layer / File(s) Summary
Guardrails configuration and validation
agents/langgraph/examples/guardrailed_agent/agent.yaml, guardrails/safety/*, tests/test_guardrails_config.py
Defines agent metadata, safety models, content and topic prompts, greeting rails, and tests for the guardrail configuration structure.
Agent runtime and API
src/guardrailed_agent/*, main.py, tests/test_main_helpers.py
Builds the banking agent and balance tool, adds optional MLflow tracing, and exposes authenticated FastAPI chat, streaming, health, playground, and image endpoints.
Playground proxy and interface
playground/app.py, playground/templates/index.html
Adds Flask proxy routes and a browser chat interface with health checks, SSE parsing, loading states, and error handling.
Local packaging and deployment workflow
Makefile, Dockerfile, pyproject.toml, values.yaml, .env.example, .gitignore, .dockerignore, README.md, tests/test_packaging_config.py
Adds dependency, container, environment, Helm, OpenShift, local development, documentation, and packaging validation configuration.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant Playground
  participant FastAPI
  participant LangGraphAgent
  participant GuardrailsProxy
  Browser->>Playground: POST /api/chat
  Playground->>FastAPI: POST /chat/completions with stream=true
  FastAPI->>LangGraphAgent: Stream agent events
  LangGraphAgent->>GuardrailsProxy: Route model traffic through BASE_URL
  GuardrailsProxy-->>FastAPI: Return guarded SSE events
  FastAPI-->>Playground: Return SSE chunks
  Playground-->>Browser: Forward streamed response
Loading

Suggested reviewers: mpk-droid

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a guardrailed banking agent example.
Description check ✅ Passed The description matches the changeset and accurately describes the new guardrailed agent example and supporting files.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Large PR detected (2315 lines changed)

This PR exceeds 1200 lines of code changes (excluding lock files, generated content, and images). Large PRs are harder to review thoroughly and are more likely to introduce bugs.

Consider splitting this PR into smaller, focused changes.

@tarun-etikala tarun-etikala changed the title feat(guardrails): add guardrailed FSI agent example (RHAIENG-6262) feat(guardrails): add guardrailed banking agent example (RHAIENG-6262) Jul 27, 2026
@tarun-etikala
tarun-etikala marked this pull request as ready for review July 27, 2026 14:34
@tarun-etikala
tarun-etikala requested a review from a team as a code owner July 27, 2026 14:34
@shricharan-ks shricharan-ks self-assigned this Jul 27, 2026

@shricharan-ks shricharan-ks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review — PR #282 (comprehensive, 2 review agents)

This PR adds a banking customer service agent with NeMo Guardrails safety layer — the first real example agent in the repo. 23 files, 5524 additions. The architecture is solid (proxy pattern with passthrough: true), the README is thorough, and the project structure correctly follows all standard conventions (UBI9, FastAPI, /chat/completions, port 8080, agent.yaml, Makefile).

Confirmed: from langchain.agents import create_agent is valid — matches the react_agent template and langchain>=1.2.10 (which resolves to 1.3.14 in the lock). Not a bug.


Findings (6 survived review)

1. 🔴 main.py:~222 — Streaming guardrails block sends empty response (CONFIRMED)

When _is_guardrails_block(e) fires in the streaming path, the code yields an empty delta {} with finish_reason: "stop" and then [DONE]. It never sends the refusal text "I'm sorry, I can't respond to that.".

The non-streaming path correctly returns _GUARDRAILS_REFUSAL as the message content. The streaming client gets silence instead of a clear refusal.

Fix: Yield a chunk with delta: {"content": _GUARDRAILS_REFUSAL} before the finish_reason: "stop" chunk:

refusal_chunk = {"choices": [{"index": 0, "delta": {"content": _GUARDRAILS_REFUSAL}, "finish_reason": None}]}
yield f"data: {json.dumps(refusal_chunk)}\n\n"
# then yield the stop chunk

2. 🟡 guardrails/safety/config.yaml — Latent secret leak via make guardrails-config (CONFIRMED)

The file commits api_key: not-needed (safe), but make guardrails-config rewrites this tracked file in-place from .env vars. If a developer runs it with a real API key for a non-Ollama endpoint and then commits, the key ends up in git history. The PR correctly notes api_key_env_var is broken in NeMo v0.21.0.

Suggestion: Either .gitignore the generated config and ship a .example template, or have make guardrails-config write to a non-tracked path (e.g., guardrails/safety/.config.yaml with .config.yaml in .gitignore).


3. 🟡 main.py:~30 — Brittle string matching for guardrails detection (PLAUSIBLE)

return isinstance(exc, OpenAIAPIError) and "Blocked by" in str(exc) and "rails" in str(exc)

The docstring acknowledges this is version-specific, but there are no unit tests exercising the detection. If NeMo changes the error message wording, guardrails blocks silently become 500 errors.

Suggestion: Add a unit test constructing a mock OpenAIAPIError with the expected message, and extract the match strings into constants.


4. 🟡 main.py:~260 — Health probe creates new httpx client per call (CONFIRMED)

Every /health call creates and tears down an httpx.AsyncClient. K8s probes hit this every 10-30s. Also, status_code < 500 treats 4xx (401/403/404) as "reachable" — a guardrails server behind auth would report as healthy even when the agent can't actually use it.

Fix: Use a module-level client and check for status_code == 200.


5. 🟡 main.py:~162 — Conversation history discarded (CONFIRMED)

_build_langchain_messages keeps only the last user message. Inherited from the react_agent template, but more impactful for a banking agent where multi-turn conversations are the norm. Also weakens guardrails — multi-message jailbreak attempts that build context across turns bypass the safety filter since prior context is dropped.

Not blocking (matches template pattern), but worth documenting as a known limitation.


6. 🟡 main.py — /health returns 503 when guardrails is unreachable (PLAUSIBLE)

If K8s uses /health as a liveness probe (not just readiness), a transient guardrails outage causes cascading pod restarts. Consider separating liveness (agent only) from readiness (agent + guardrails), or documenting that /health should only be a readiness probe.


Additional observations (not blocking)

  • nemoguardrails==0.21.0 is pinned 2 versions behind latest (0.23.0) to match RHOAI — add an inline comment explaining the pin
  • langchain-community>=0.2,<0.4 is capped below the final archived release (0.4.2) — driven by NeMo dependency, worth a comment
  • protobuf<7 is unique to this agent and likely a transitive NeMo conflict — document origin
  • Dockerfile correctly omits [guardrails] extra (NeMo runs as separate server) but a comment would prevent confusion

What looks good

  • First example agent — sets an excellent precedent for the examples/ pattern
  • Proxy pattern (passthrough: true) is the right architecture — NeMo as a pure safety filter without running its conversational engine
  • Three-layer input rails (regex → content safety → topic safety) provide defense in depth
  • Error handling catches NeMo-specific exceptions in both streaming and non-streaming paths
  • Dockerfile follows all conventions (UBI9, UID 1001, port 8080, digest-pinned base)
  • 29 structural tests validate guardrails config and packaging
  • Clean separation of concerns — agent doesn't import NeMo, config is external

Summary

Finding #1 (streaming refusal) is the most impactful — users get silence instead of a refusal for blocked streaming requests. Fix that, consider the config.yaml secret-leak vector (#2), and this is a strong first example agent ready to merge.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 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.

Inline comments:
In `@agents/langgraph/examples/guardrailed_agent/LICENSE`:
- Around line 1-3: Expand the LICENSE file’s existing MIT License header and
copyright notice with the complete standard MIT license text, including the
permission grant, conditions, and liability disclaimer.

In `@agents/langgraph/examples/guardrailed_agent/main.py`:
- Around line 174-193: Load the dotenv configuration near the top of the module
before invoking _configure_auth_middleware(), so _auth_enabled() reads
AUTH_ENABLED values from .env. Keep the existing middleware configuration and
tracing behavior unchanged.
- Around line 196-201: Update _build_langchain_messages to convert the complete
messages list into LangChain messages, preserving all user, assistant, and
system turns in their original order instead of returning only the last user
message. Import and use the appropriate BaseMessage subclasses, while retaining
the existing error behavior for requests without a user message.
- Around line 456-473: Update the guardrails-block branch in the streaming
exception handler to emit the same _GUARDRAILS_REFUSAL message used by
_handle_chat before the terminal stop chunk. Include the refusal text in the
streamed content delta while preserving the existing finish_reason and [DONE]
signaling.
- Around line 260-319: Update the ChatCompletionResponse model used by
_handle_chat to declare the returned context field, preserving context_messages
in the /chat/completions response; alternatively remove the context construction
and response entry if it is not part of the intended API.

In `@agents/langgraph/examples/guardrailed_agent/Makefile`:
- Around line 37-38: Replace the remote pipe-to-shell command in the Makefile’s
Ollama installation step with a pinned, checksummed release artifact workflow,
or remove execution and document the official installation command for
developers to run manually.
- Around line 142-146: Update the Helm secret generation flow near the Makefile
recipe’s temporary .helm-secrets.yaml creation to avoid manually interpolating
API_KEY and MLFLOW_TRACKING_TOKEN into YAML. Use Helm’s --set-file with securely
created temporary files, or an existing real YAML serializer, so quotes,
backslashes, and newlines are preserved exactly; retain cleanup and restrictive
permissions for all temporary secret files.

In `@agents/langgraph/examples/guardrailed_agent/playground/app.py`:
- Line 35: Update the IMAGES_DIR path calculation in app.py to walk up one
additional parent directory, resolving the repository root before appending
images; leave the /images route behavior unchanged.
- Around line 54-95: Flask proxy endpoints do not match the shared template’s
hardcoded paths. In
agents/langgraph/examples/guardrailed_agent/playground/app.py lines 54-95,
rename the health and chat routes in health and chat to /health and
/chat/completions (or proxy those exact paths);
agents/langgraph/examples/guardrailed_agent/playground/templates/index.html
lines 217-232 and 314-332 require no direct changes.

In `@agents/langgraph/examples/guardrailed_agent/pyproject.toml`:
- Around line 8-38: Add Flask and requests to the declared package dependencies
for the guardrailed agent project, since playground/app.py imports both
unconditionally. Update the dependencies list while preserving the existing
version constraints and optional extras.

In `@agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/agent.py`:
- Around line 29-44: In the startup validation flow, add an explicit check that
model_id is set after resolving the argument and MODEL_ID environment variable.
Raise a clear ValueError identifying MODEL_ID and explaining how to provide it,
matching the existing base_url and api_key validation style.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 2387ae03-b23e-45ef-86a2-30ee029ee710

📥 Commits

Reviewing files that changed from the base of the PR and between c26707b and aa141ca.

⛔ Files ignored due to path filters (1)
  • agents/langgraph/examples/guardrailed_agent/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • agents/langgraph/examples/guardrailed_agent/.dockerignore
  • agents/langgraph/examples/guardrailed_agent/.env.example
  • agents/langgraph/examples/guardrailed_agent/Dockerfile
  • agents/langgraph/examples/guardrailed_agent/LICENSE
  • agents/langgraph/examples/guardrailed_agent/Makefile
  • agents/langgraph/examples/guardrailed_agent/README.md
  • agents/langgraph/examples/guardrailed_agent/agent.yaml
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/config.yaml
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/prompts.yml
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/rails.co
  • agents/langgraph/examples/guardrailed_agent/main.py
  • agents/langgraph/examples/guardrailed_agent/playground/app.py
  • agents/langgraph/examples/guardrailed_agent/playground/templates/index.html
  • agents/langgraph/examples/guardrailed_agent/pyproject.toml
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/__init__.py
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/agent.py
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/tools.py
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/tracing.py
  • agents/langgraph/examples/guardrailed_agent/tests/__init__.py
  • agents/langgraph/examples/guardrailed_agent/tests/test_guardrails_config.py
  • agents/langgraph/examples/guardrailed_agent/tests/test_packaging_config.py
  • agents/langgraph/examples/guardrailed_agent/values.yaml

Comment on lines +142 to +146
trap 'rm -f .helm-secrets.yaml' EXIT && \
umask 077 && \
{ printf 'secrets:\n apiKey: "%s"\n' "$${API_KEY}"; \
[ -z "$${MLFLOW_TRACKING_TOKEN}" ] || printf ' mlflowTrackingToken: "%s"\n' "$${MLFLOW_TRACKING_TOKEN}"; \
} > .helm-secrets.yaml && \

@coderabbitai coderabbitai Bot Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Serialize Helm secrets without manual YAML interpolation.

API keys and MLflow tokens may legally contain ", \, or newlines; printf inserts them unescaped into .helm-secrets.yaml, producing invalid or changed secret values. Use Helm --set-file with temporary secret files, or a real YAML serializer.

🤖 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 `@agents/langgraph/examples/guardrailed_agent/Makefile` around lines 142 - 146,
Update the Helm secret generation flow near the Makefile recipe’s temporary
.helm-secrets.yaml creation to avoid manually interpolating API_KEY and
MLFLOW_TRACKING_TOKEN into YAML. Use Helm’s --set-file with securely created
temporary files, or an existing real YAML serializer, so quotes, backslashes,
and newlines are preserved exactly; retain cleanup and restrictive permissions
for all temporary secret files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged — same pattern in all agents' Makefiles across the repo. Will track as a cross-agent improvement separately.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tarun-etikala, acknowledged. The finding remains valid but is reasonable to address as a cross-agent follow-up rather than in this PR. Would you like me to open a GitHub issue capturing the affected Makefile pattern and acceptance criteria?

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@agents/langgraph/examples/guardrailed_agent/.gitignore`:
- Around line 1-3: Add an explicit .env entry to the .gitignore alongside
guardrails/safety/config.yaml, ensuring the generated environment file created
by make init is ignored and API_KEY credentials cannot be committed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 84a9b45a-3ebe-4173-bd5e-bce9ef428809

📥 Commits

Reviewing files that changed from the base of the PR and between aa141ca and 00ce4dc.

⛔ Files ignored due to path filters (1)
  • agents/langgraph/examples/guardrailed_agent/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • agents/langgraph/examples/guardrailed_agent/.env.example
  • agents/langgraph/examples/guardrailed_agent/.gitignore
  • agents/langgraph/examples/guardrailed_agent/Makefile
  • agents/langgraph/examples/guardrailed_agent/README.md
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/config.yaml.example
  • agents/langgraph/examples/guardrailed_agent/main.py
  • agents/langgraph/examples/guardrailed_agent/playground/app.py
  • agents/langgraph/examples/guardrailed_agent/pyproject.toml
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/agent.py
  • agents/langgraph/examples/guardrailed_agent/tests/test_guardrails_config.py
  • agents/langgraph/examples/guardrailed_agent/tests/test_main_helpers.py
🚧 Files skipped from review as they are similar to previous changes (5)
  • agents/langgraph/examples/guardrailed_agent/.env.example
  • agents/langgraph/examples/guardrailed_agent/pyproject.toml
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/agent.py
  • agents/langgraph/examples/guardrailed_agent/playground/app.py
  • agents/langgraph/examples/guardrailed_agent/Makefile

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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.

Inline comments:
In `@agents/langgraph/examples/guardrailed_agent/Makefile`:
- Line 8: Update the .PHONY declaration to include the _check-env target,
ensuring deploy always executes its environment validation even when a file
named _check-env exists.
- Around line 17-18: Update the Makefile init target’s .env creation command to
enforce owner-only permissions when copying .env.example, using install -m 600
or an equivalent restrictive umask while preserving the existing skip behavior
when .env already exists.
- Around line 59-70: Update the guardrails-server recipe’s embedded Python
command to import os and read API_KEY, LLM_BASE_URL, and MODEL_ID via
os.environ.get() with the existing defaults, removing their shell interpolation
from the Python source. Preserve the current YAML model updates and avoid
exposing secrets in process arguments.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 89a5a186-85ac-4858-9d84-09e9c8de7dec

📥 Commits

Reviewing files that changed from the base of the PR and between 00ce4dc and d790328.

📒 Files selected for processing (5)
  • agents/langgraph/examples/guardrailed_agent/.env.example
  • agents/langgraph/examples/guardrailed_agent/.gitignore
  • agents/langgraph/examples/guardrailed_agent/Makefile
  • agents/langgraph/examples/guardrailed_agent/README.md
  • agents/langgraph/examples/guardrailed_agent/tests/test_packaging_config.py
🚧 Files skipped from review as they are similar to previous changes (4)
  • agents/langgraph/examples/guardrailed_agent/.gitignore
  • agents/langgraph/examples/guardrailed_agent/.env.example
  • agents/langgraph/examples/guardrailed_agent/README.md
  • agents/langgraph/examples/guardrailed_agent/tests/test_packaging_config.py

CONTAINER_CLI := $(shell command -v podman 2>/dev/null || command -v docker 2>/dev/null)
MODEL ?= llama3.1:8b

.PHONY: init re-init env ollama run-app run-app-fresh build push build-openshift deploy undeploy test dry-run help guardrails-server

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Mark _check-env as phony.

deploy depends on _check-env, but this target is absent from .PHONY. If a file named _check-env exists, make can skip the required environment validation and continue with missing deployment settings. Add _check-env to the .PHONY list.

🤖 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 `@agents/langgraph/examples/guardrailed_agent/Makefile` at line 8, Update the
.PHONY declaration to include the _check-env target, ensuring deploy always
executes its environment validation even when a file named _check-env exists.

@tarun-etikala

Copy link
Copy Markdown
Contributor Author

Review Response — @shricharan-ks findings

Thanks for the thorough review! All 6 findings have been addressed across commits 00ce4dc, d790328, and c9d1080:

Fixed

# Finding Fix
1 🔴 Streaming block sends empty response Added _GUARDRAILS_REFUSAL content chunk before finish_reason: stop in streaming handler. Verified E2E — streaming toxic input now returns refusal text.
2 🟡 Config.yaml secret leak Renamed to config.yaml.example (committed template), gitignored runtime copy. Then went further — folded make guardrails-config into make guardrails-server so config is generated fresh from .env at each startup. Secrets never persist in the working tree.
3 🟡 Brittle string matching, no tests Added 7 unit tests for _is_guardrails_block() covering all match/non-match cases (content safety, topic safety, partial matches, non-OpenAI errors).
4 🟡 Health probe: new client per call, 4xx = reachable Module-level httpx.AsyncClient, check status_code == 200.

Documented as known limitations

# Finding Action
5 🟡 Conversation history discarded Documented in README Known Limitations section. Pre-existing template pattern (same in react_agent).
6 🟡 /health 503 causes pod restarts Documented in README: use as readiness probe, not liveness.

Additional observations

All 4 inline comment suggestions addressed in c9d1080:

  • nemoguardrails==0.21.0 — comment: "pinned to match RHOAI container version"
  • langchain-community<0.4 — comment: ">=0.4 drops GoogleSearchAPIWrapper, breaks nemoguardrails imports"
  • protobuf<7 — comment: "nemoguardrails 0.21.0 pulls grpcio which conflicts with protobuf 7+"
  • Dockerfile omits [guardrails] — comment: "NeMo runs as a separate pod in production (managed by NemoGuardrails CRD)"

tarun-etikala and others added 14 commits July 28, 2026 14:57
Copy main.py, tracing.py, playground, LICENSE, .dockerignore from
react_agent template. Fix imports to guardrailed_agent package.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… tool (RHAIENG-6262)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Layered input rails: regex (instant) -> content safety (S1-S13) ->
topic safety (banking domain). Output rail: content safety check.
Streaming enabled. Self-check classification (main LLM judges safety).
config.yaml (not .yml) + rails.co required by RHOAI entrypoint.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nemoguardrails[server]==0.21.0 pinned as optional 'guardrails' extra.
BASE_URL defaults to NeMo proxy (localhost:8090/v1).
Agent env vars: GUARDRAILS_PORT, LLM_BASE_URL for local dev.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Makefile adds guardrails-config (update config from .env) and
guardrails-server (start NeMo proxy on port 8090).
env target installs nemoguardrails via --extra guardrails.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Structural tests validate guardrails config files (YAML well-formed,
required keys, correct output parsers, streaming enabled) and
packaging config (Makefile targets, Dockerfile paths, nemoguardrails
pin). All 28 tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers architecture (proxy pattern), local dev setup (Ollama + NeMo +
agent), guardrails test commands, use-case swapping, OpenShift deploy,
and guardrails config reference.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nemoguardrails 0.21.0 imports dataclasses_json internally but does
not declare it as a dependency. Add it explicitly to the guardrails
optional group.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pin langchain-community<0.4 to fix GoogleSearchAPIWrapper warnings
from nemoguardrails 0.21.0 (removed in 0.4.x). Clarify README step
3: separate model pull from ollama serve, add macOS app note.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Switch guardrails config default from Ollama (11434) to OGX (8321)
for consistency with react_agent workflow. Update README to use
make ogx-server instead of ollama serve. Update MODEL_ID to use
ollama/ prefix matching OGX convention.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
OGX model names are case-sensitive and don't use the ollama/ prefix.
Change from 'ollama/Llama3.1:8B' to 'llama3.1:8b' in guardrails
config and Makefile default.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Enable passthrough: true so NeMo acts as a safety filter, not a
conversational engine. Without it, NeMo runs 3 extra LLM calls
(intent/next-step/response generation) and ignores the agent's
system prompt and tools. With passthrough, the original prompt
passes straight through — rails still fire on input and output.
Switch from api_key_env_var to api_key in parameters (v0.21.0
LangChain initializer doesn't read api_key_env_var correctly).

E2E verified: on-topic passes, toxic blocked, off-topic blocked,
greeting passes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Point guardrails config and .env defaults at Ollama (11434) instead
of OGX (8321). OGX has a pre-existing model name case mismatch
(Llama3.1:8B vs llama3.1:8b) that's a separate repo-wide issue.
Enable passthrough mode, use api_key in parameters.
Simplify README to 6 steps (remove OGX step).

E2E verified from clean make init: on-topic passes, toxic blocked,
off-topic blocked, greeting passes. 28/28 unit tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tarun-etikala and others added 11 commits July 28, 2026 14:57
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove unused ogx-server target and .PHONY entry. Fix
guardrails-config default LLM_BASE_URL to Ollama (11434).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When topic safety input rail misses an off-topic request (model
dependent), the LLM may generate a response that the output content
safety rail then blocks. NeMo returns an error that ChatOpenAI raises
as APIError. Catch guardrails block errors and return a clean refusal
instead of HTTP 500.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When NeMo output rail blocks a response mid-stream, the agent gets
an openai.APIError. Handle it in _handle_stream by sending a clean
finish chunk and [DONE] (NeMo already sent the refusal message via
the stream before the error). Prevents duplicate refusal messages
and Internal server error in streaming mode.

E2E verified: streaming off-topic 3/3, streaming on-topic, non-stream
toxic, non-stream greeting — all clean, zero 500s in logs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
P1.2: Add max_tokens: 10 to topic safety prompt to cap LLM output.
P2.3: Remove unused AccountInput class from tools.py.
P2.5: Add api_key sed replacement to guardrails-config target.
P3.6: Extract _is_guardrails_block() helper, narrow to OpenAIAPIError.
P3.7: Log exceptions server-side, return generic message to client.
P3.10: Use NeMo refusal message from exception when available.

Also: remove unused deps (milvus-lite, pymilvus, pypdf, flask,
chardet), fix Python version <3.14, fix FastAPI title.

E2E verified: 8/8 tests (streaming + non-streaming), zero errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
F1: Use _GUARDRAILS_REFUSAL constant (not NeMo internal rail names).
F2: Fix playground images path (4 parents, not 3).
F3: Add guardrails_reachable probe to /health endpoint.
F4: Replace sed with python3+yaml in guardrails-config target.
F5: Document credentials warning for make guardrails-config.
F6: Add test_passthrough_enabled assertion.
F7: Add topic_control model type assertion.

E2E verified: health probe (up/down), 5 request tests, 29 unit tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove test-integration and test-auth-integration — no
tests/integration/ directory exists. Simplify test target.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Send refusal text in streaming block path (was empty response)
- Module-level httpx client for health probe, check status_code == 200
- Add model_id validation in agent.py
- Gitignore config.yaml, ship config.yaml.example template, make init copies it
- Fix playground/app.py IMAGES_DIR path (parents[4] → parents[5])
- Add flask/requests back to deps (playground imports them)
- Add unit tests for _is_guardrails_block() helper
- Remove MIT LICENSE stub (repo uses Apache-2.0)
- Document known limitations in README

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…p (RHAIENG-6262)

Fold `make guardrails-config` into `make guardrails-server` so config.yaml
is generated fresh from .env each time the server starts. Secrets never
persist in the working tree between runs. Also add .env to .gitignore.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Explain why nemoguardrails==0.21.0, langchain-community<0.4, and
protobuf<7 are pinned. Note why Dockerfile omits [guardrails] extra.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kami619
kami619 force-pushed the rhaieng-6262-guardrailed-agent branch from c9d1080 to 847d8f2 Compare July 28, 2026 09:28

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (3)
agents/langgraph/examples/guardrailed_agent/Makefile (3)

8-8: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

_check-env still missing from .PHONY.

deploy/dry-run depend on _check-env; if a file with that name ever exists, make could skip env validation.

diff
-.PHONY: init re-init env ollama run-app run-app-fresh build push build-openshift deploy undeploy test dry-run help guardrails-server
+.PHONY: init re-init env ollama run-app run-app-fresh build push build-openshift deploy undeploy test dry-run help guardrails-server _check-env
🤖 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 `@agents/langgraph/examples/guardrailed_agent/Makefile` at line 8, Add
_check-env to the .PHONY target declaration so deploy and dry-run always execute
environment validation regardless of whether a file with that name exists.

62-71: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Secrets still shell-interpolated into the embedded python -c source.

API_KEY, LLM_BASE_URL, MODEL_ID are expanded by the shell before reaching Python; quotes/backslashes/newlines can corrupt the generated code or inject Python, and API_KEY ends up in process arguments.

diff
-	python3 -c "\
-import yaml; \
+	python3 -c "\
+import os, yaml; \
 f='guardrails/safety/config.yaml'; \
 c=yaml.safe_load(open(f)); \
-url='$${LLM_BASE_URL:-http://localhost:11434/v1}'; \
-model='$${MODEL_ID:-llama3.1:8b}'; \
-key='$${API_KEY:-not-needed}'; \
+url=os.environ.get('LLM_BASE_URL') or 'http://localhost:11434/v1'; \
+model=os.environ.get('MODEL_ID') or 'llama3.1:8b'; \
+key=os.environ.get('API_KEY') or 'not-needed'; \
🤖 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 `@agents/langgraph/examples/guardrailed_agent/Makefile` around lines 62 - 71,
Update the Makefile’s embedded Python configuration generation to read
LLM_BASE_URL, MODEL_ID, and API_KEY from the runtime environment rather than
interpolating their values into the python -c source. Preserve the existing
defaults and model updates, while ensuring secrets are not exposed through
generated source or process arguments.

17-18: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

.env still created without owner-only permissions.

cp doesn't restrict permissions; .env will contain API_KEY.

diff
-	`@if` [ ! -f .env ]; then cp .env.example .env && echo "Created .env from .env.example - edit it with your configuration"; else echo ".env already exists - skipping"; fi
+	`@if` [ ! -f .env ]; then install -m 600 .env.example .env && echo "Created .env from .env.example - edit it with your configuration"; else echo ".env already exists - skipping"; fi
🤖 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 `@agents/langgraph/examples/guardrailed_agent/Makefile` around lines 17 - 18,
The init target’s .env creation via cp leaves configuration secrets accessible
under default permissions. Update the init recipe to create .env with owner-only
permissions (such as 600) while preserving the existing conditional behavior and
messages.
🤖 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.

Inline comments:
In `@agents/langgraph/examples/guardrailed_agent/main.py`:
- Around line 304-321: The response builds and returns context_messages, but
ChatCompletionResponse strips the undeclared context field. Update
ChatCompletionResponse to declare a context field with the appropriate type so
/chat/completions responses preserve it, or remove context_messages construction
and the returned context entry if context is not part of the API contract.
- Around line 176-195: The auth decision in _configure_auth_middleware is
executed before the .env-loading path in lifespan via enable_tracing, so
AUTH_ENABLED values from .env are missed. Ensure environment loading occurs
before _configure_auth_middleware runs, or move the middleware configuration
into the initialized lifespan flow after enable_tracing, while preserving the
existing dependency error handling and avoiding duplicate middleware
registration.

In `@agents/langgraph/examples/guardrailed_agent/Makefile`:
- Around line 142-144: The Makefile’s Helm secret generation block still
interpolates API_KEY and MLFLOW_TRACKING_TOKEN directly into YAML via printf.
Replace the manual serialization in the target recipe with Helm --set-file using
securely created temporary files (or an existing YAML serializer), preserving
omission of MLFLOW_TRACKING_TOKEN when unset and ensuring both values remain
valid for quotes, backslashes, and newlines.

In `@agents/langgraph/examples/guardrailed_agent/playground/app.py`:
- Around line 54-95: The Flask route decorators for health and chat use
/api/health and /api/chat, while the shared template requests /health and
/chat/completions. Update the routes on health and chat to those exact endpoint
paths, preserving their existing handler implementations and streaming behavior.
- Line 32: Update logging configuration in the application initialization and
the corresponding chunk logging near the later occurrence so chat content is not
emitted in deployments by default: gate DEBUG level behind the existing
FLASK_DEBUG environment setting, or remove the message content from the
logger.debug call while preserving useful diagnostics.

In `@agents/langgraph/examples/guardrailed_agent/tests/test_packaging_config.py`:
- Around line 65-67: Update test_pyproject_pins_nemoguardrails_version to assert
that the normalized guardrails dependency entry exactly matches the required
nemoguardrails server extra and 0.21.0 pin, rather than using substring checks;
preserve validation against the guardrails optional-dependencies list.

---

Duplicate comments:
In `@agents/langgraph/examples/guardrailed_agent/Makefile`:
- Line 8: Add _check-env to the .PHONY target declaration so deploy and dry-run
always execute environment validation regardless of whether a file with that
name exists.
- Around line 62-71: Update the Makefile’s embedded Python configuration
generation to read LLM_BASE_URL, MODEL_ID, and API_KEY from the runtime
environment rather than interpolating their values into the python -c source.
Preserve the existing defaults and model updates, while ensuring secrets are not
exposed through generated source or process arguments.
- Around line 17-18: The init target’s .env creation via cp leaves configuration
secrets accessible under default permissions. Update the init recipe to create
.env with owner-only permissions (such as 600) while preserving the existing
conditional behavior and messages.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: be924480-d137-4f84-829d-1cd4647006db

📥 Commits

Reviewing files that changed from the base of the PR and between c9d1080 and 847d8f2.

⛔ Files ignored due to path filters (1)
  • agents/langgraph/examples/guardrailed_agent/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (23)
  • agents/langgraph/examples/guardrailed_agent/.dockerignore
  • agents/langgraph/examples/guardrailed_agent/.env.example
  • agents/langgraph/examples/guardrailed_agent/.gitignore
  • agents/langgraph/examples/guardrailed_agent/Dockerfile
  • agents/langgraph/examples/guardrailed_agent/Makefile
  • agents/langgraph/examples/guardrailed_agent/README.md
  • agents/langgraph/examples/guardrailed_agent/agent.yaml
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/config.yaml.example
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/prompts.yml
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/rails.co
  • agents/langgraph/examples/guardrailed_agent/main.py
  • agents/langgraph/examples/guardrailed_agent/playground/app.py
  • agents/langgraph/examples/guardrailed_agent/playground/templates/index.html
  • agents/langgraph/examples/guardrailed_agent/pyproject.toml
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/__init__.py
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/agent.py
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/tools.py
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/tracing.py
  • agents/langgraph/examples/guardrailed_agent/tests/__init__.py
  • agents/langgraph/examples/guardrailed_agent/tests/test_guardrails_config.py
  • agents/langgraph/examples/guardrailed_agent/tests/test_main_helpers.py
  • agents/langgraph/examples/guardrailed_agent/tests/test_packaging_config.py
  • agents/langgraph/examples/guardrailed_agent/values.yaml
🚧 Files skipped from review as they are similar to previous changes (14)
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/tools.py
  • agents/langgraph/examples/guardrailed_agent/.gitignore
  • agents/langgraph/examples/guardrailed_agent/agent.yaml
  • agents/langgraph/examples/guardrailed_agent/.dockerignore
  • agents/langgraph/examples/guardrailed_agent/.env.example
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/rails.co
  • agents/langgraph/examples/guardrailed_agent/tests/test_main_helpers.py
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/prompts.yml
  • agents/langgraph/examples/guardrailed_agent/guardrails/safety/config.yaml.example
  • agents/langgraph/examples/guardrailed_agent/values.yaml
  • agents/langgraph/examples/guardrailed_agent/src/guardrailed_agent/agent.py
  • agents/langgraph/examples/guardrailed_agent/pyproject.toml
  • agents/langgraph/examples/guardrailed_agent/tests/test_guardrails_config.py
  • agents/langgraph/examples/guardrailed_agent/Dockerfile

Comment on lines +176 to +195
def _auth_enabled() -> bool:
return getenv("AUTH_ENABLED", "false").strip().lower() == "true"


def _configure_auth_middleware() -> None:
if not _auth_enabled():
return

try:
from agent_auth.middleware import SATokenAuthMiddleware
except ModuleNotFoundError as exc:
raise RuntimeError(
"AUTH_ENABLED=true, but auth middleware dependencies are not installed. "
"Run `uv sync --extra auth` to enable ServiceAccount token auth locally."
) from exc

app.add_middleware(SATokenAuthMiddleware)


_configure_auth_middleware()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Auth middleware decision still made before .env is loaded.

_configure_auth_middleware() runs at import time, but .env is only loaded later via enable_tracing() inside lifespan(). If AUTH_ENABLED=true is set only in .env (not exported), _auth_enabled() reads an empty os.environ and the auth middleware is silently never installed.

🤖 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 `@agents/langgraph/examples/guardrailed_agent/main.py` around lines 176 - 195,
The auth decision in _configure_auth_middleware is executed before the
.env-loading path in lifespan via enable_tracing, so AUTH_ENABLED values from
.env are missed. Ensure environment loading occurs before
_configure_auth_middleware runs, or move the middleware configuration into the
initialized lifespan flow after enable_tracing, while preserving the existing
dependency error handling and avoiding duplicate middleware registration.

Comment on lines +304 to +321
return {
"id": _make_completion_id(),
"object": "chat.completion",
"created": int(time.time()),
"model": model_id,
"choices": [
{
"index": 0,
"message": {
"role": "assistant",
"content": assistant_content,
},
"finish_reason": "stop",
}
],
"context": context_messages,
"usage": _extract_usage(result.get("messages", [])),
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

context is still computed and returned but stripped by response_model=ChatCompletionResponse.

context_messages is built and included in the returned dict, but ChatCompletionResponse has no context field, so FastAPI silently drops it from every /chat/completions response. Either add the field to the model or stop building/returning it.

🤖 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 `@agents/langgraph/examples/guardrailed_agent/main.py` around lines 304 - 321,
The response builds and returns context_messages, but ChatCompletionResponse
strips the undeclared context field. Update ChatCompletionResponse to declare a
context field with the appropriate type so /chat/completions responses preserve
it, or remove context_messages construction and the returned context entry if
context is not part of the API contract.

Comment on lines +142 to +144
{ printf 'secrets:\n apiKey: "%s"\n' "$${API_KEY}"; \
[ -z "$${MLFLOW_TRACKING_TOKEN}" ] || printf ' mlflowTrackingToken: "%s"\n' "$${MLFLOW_TRACKING_TOKEN}"; \
} > .helm-secrets.yaml && \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Helm secrets still serialized via manual printf interpolation.

API_KEY/MLFLOW_TRACKING_TOKEN may contain ", \, or newlines, which printf won't escape, producing an invalid/corrupted .helm-secrets.yaml. Prefer --set-file with temp files or a real YAML serializer.

🤖 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 `@agents/langgraph/examples/guardrailed_agent/Makefile` around lines 142 - 144,
The Makefile’s Helm secret generation block still interpolates API_KEY and
MLFLOW_TRACKING_TOKEN directly into YAML via printf. Replace the manual
serialization in the target recipe with Helm --set-file using securely created
temporary files (or an existing YAML serializer), preserving omission of
MLFLOW_TRACKING_TOKEN when unset and ensuring both values remain valid for
quotes, backslashes, and newlines.

stream_with_context,
)

logging.basicConfig(level=logging.DEBUG)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Unconditional DEBUG logging captures chat content.

logging.basicConfig(level=logging.DEBUG) is hardcoded (independent of FLASK_DEBUG), so logger.debug(f"Chunk: {chunk[:200]}") will log user/assistant message content to server logs in any deployment. Gate log level behind an env var, or drop the content from the debug log line.

diff
-logging.basicConfig(level=logging.DEBUG)
+logging.basicConfig(level=getenv("LOG_LEVEL", "INFO").upper())

Also applies to: 113-113

🤖 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 `@agents/langgraph/examples/guardrailed_agent/playground/app.py` at line 32,
Update logging configuration in the application initialization and the
corresponding chunk logging near the later occurrence so chat content is not
emitted in deployments by default: gate DEBUG level behind the existing
FLASK_DEBUG environment setting, or remove the message content from the
logger.debug call while preserving useful diagnostics.

Comment on lines +54 to +95
@app.route("/api/health", methods=["GET"])
def health():
"""Check if the agent is reachable."""
try:
resp = http_requests.get(f"{AGENT_URL}/health", timeout=5)
return jsonify(resp.json()), resp.status_code
except Exception:
logger.exception("Error checking agent health")
return (
jsonify(
{
"status": "unreachable",
"error": "Agent is unreachable. Please try again later.",
}
),
503,
)


@app.route("/api/chat", methods=["POST"])
def chat():
"""Proxy chat requests to the agent with streaming."""
data = request.get_json() or {}
messages = data.get("messages", [])

payload = {
"messages": messages,
"stream": True,
}

logger.info(
f"Sending request to {AGENT_URL}/chat/completions (messages={len(payload.get('messages', []))}, stream={payload.get('stream')})"
)

def generate():
try:
with http_requests.post(
f"{AGENT_URL}/chat/completions",
json=payload,
stream=True,
timeout=(10, 300),
) as resp:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Flask routes still don't match the shared template's hardcoded endpoints — playground remains non-functional standalone.

index.html fetches /health and /chat/completions, but this file still serves /api/health and /api/chat. Every health check and chat request through this Flask playground 404s.

🤖 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 `@agents/langgraph/examples/guardrailed_agent/playground/app.py` around lines
54 - 95, The Flask route decorators for health and chat use /api/health and
/api/chat, while the shared template requests /health and /chat/completions.
Update the routes on health and chat to those exact endpoint paths, preserving
their existing handler implementations and streaming behavior.

Comment on lines +65 to +67
def test_pyproject_pins_nemoguardrails_version() -> None:
guardrails_deps = PYPROJECT["project"]["optional-dependencies"]["guardrails"]
assert any("nemoguardrails" in d and "0.21.0" in d for d in guardrails_deps)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Enforce the exact NeMo Guardrails dependency pin.

This assertion also passes for nemoguardrails==0.21.0 (missing the required [server] extra) or an unpinned specifier such as nemoguardrails>=0.21.0. Compare the normalized dependency entry exactly so the server extra and pin cannot regress while this test remains green.

Proposed fix
-    assert any("nemoguardrails" in d and "0.21.0" in d for d in guardrails_deps)
+    assert "nemoguardrails[server]==0.21.0" in guardrails_deps
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_pyproject_pins_nemoguardrails_version() -> None:
guardrails_deps = PYPROJECT["project"]["optional-dependencies"]["guardrails"]
assert any("nemoguardrails" in d and "0.21.0" in d for d in guardrails_deps)
def test_pyproject_pins_nemoguardrails_version() -> None:
guardrails_deps = PYPROJECT["project"]["optional-dependencies"]["guardrails"]
assert "nemoguardrails[server]==0.21.0" in guardrails_deps
🤖 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 `@agents/langgraph/examples/guardrailed_agent/tests/test_packaging_config.py`
around lines 65 - 67, Update test_pyproject_pins_nemoguardrails_version to
assert that the normalized guardrails dependency entry exactly matches the
required nemoguardrails server extra and 0.21.0 pin, rather than using substring
checks; preserve validation against the guardrails optional-dependencies list.

@shricharan-ks shricharan-ks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — All findings resolved ✅

The latest commits address all 6 findings from the previous review:

Finding 1 (Streaming refusal sends empty delta) — FIXED ✅

Now sends delta: {"content": _GUARDRAILS_REFUSAL} before the stop chunk:

refusal_data = {"choices": [{"delta": {"content": _GUARDRAILS_REFUSAL}, "finish_reason": None}]}
yield f"data: {json.dumps(refusal_data)}\n\n"

Finding 2 (config.yaml latent secret leak) — FIXED ✅

config.yaml is now gitignored. Shipped as config.yaml.example, generated at startup by make guardrails-server. README: "The generated config.yaml is gitignored to prevent accidental credential commits."

Finding 3 (Brittle string matching, no tests) — FIXED ✅

Unit tests added for _is_guardrails_block covering:

  • content_safety_check_input rails block
  • content_safety_check_output rails block
  • topic_safety_check_input rails block
  • Generic OpenAI error (negative case)

Finding 4 (Health probe creates new client, 4xx treated as healthy) — FIXED ✅

  • Module-level _health_client = httpx.AsyncClient(timeout=2) — reuses across probes
  • Changed from status_code < 500 to status_code == 200 — 4xx no longer masks auth/routing issues

Finding 5 (Conversation history discarded) — Acknowledged ✅

Inherited from template pattern, documented as known limitation.

Finding 6 (503 on guardrails outage risks pod restarts) — Documented ✅

README now states: "Use /health as a Kubernetes readiness probe, not a liveness probe, to avoid unnecessary pod restarts."

Additional improvements in latest commits

  • make guardrails-server now generates config from .env at startup — no separate make guardrails-config step
  • Inline comments in pyproject.toml explaining pinned deps
  • Dockerfile comment explaining why [guardrails] extra is omitted

LGTM — ready to merge.

@tarun-etikala
tarun-etikala merged commit 2724d4b into red-hat-data-services:main Jul 28, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants