feat(server): add /v1/guardrail/checks endpoint#2013
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR adds a
|
| Filename | Overview |
|---|---|
| nemoguardrails/server/api.py | Adds /v1/guardrail/checks endpoint with helpers _inject_model, _filter_log, _map_rail_status, _build_rails_status; refactors model injection into a shared _inject_model function. |
| nemoguardrails/server/schemas/openai.py | Adds GuardrailCheckRequest, GuardrailCheckResponse, GuardrailCheckDataInput, GuardrailCheckDataOutput, and RailStatusEntry schemas; config field exclusivity is validated correctly. |
| nemoguardrails/rails/llm/options.py | Adds optional log: GenerationLog field to RailsResult; minimal, backward-compatible change. |
| nemoguardrails/rails/llm/llmrails.py | Threads log=response.log into all three RailsResult return sites in check_async; no logic changes. |
| tests/server/test_guardrail_checks.py | 16 tests covering status mapping, config resolution paths, validation errors, context forwarding, and log filtering; good coverage. |
Sequence Diagram
sequenceDiagram
participant C as Client
participant A as api.py /v1/guardrail/checks
participant R as _get_rails / LLMRails (inline)
participant CA as llmrails.check_async
participant GA as llmrails.generate_async
C->>A: POST /v1/guardrail/checks (model, messages, guardrails)
A->>A: validate messages non-empty
alt inline dict config
A->>R: RailsConfig.from_content(config) + _inject_model
R-->>A: LLMRails instance (fresh)
else config_id / config_ids / default
A->>R: _get_rails(config_ids, model_name)
R-->>A: LLMRails instance (cached)
end
A->>CA: check_async(messages)
CA->>CA: _determine_rails_from_messages
CA->>GA: generate_async(messages, options)
GA-->>CA: GenerationResponse (with log)
CA-->>A: RailsResult(status, content, rail, log)
A->>A: _build_rails_status(result)
A->>A: _filter_log(result.log.model_dump(), log_options)
A-->>C: GuardrailCheckResponse(status, rails_status, guardrails_data)
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
nemoguardrails/server/api.py:735
`model_dump()` without `mode="json"` may produce non-JSON-serializable objects
`result.log.model_dump()` uses the default `mode="python"`, which keeps Python objects (e.g. Enum instances, datetime values, or arbitrary `return_value: Any` objects inside `ExecutedAction`) in the resulting dict. That dict is then stored in `GuardrailCheckDataOutput.log: Optional[dict]` — a plain `dict` field with no further Pydantic coercion. FastAPI's `jsonable_encoder` will attempt standard JSON serialization on these raw Python objects, which can raise a `TypeError` at response time if any rail action returns a non-primitive value. Using `model_dump(mode="json")` ensures all values are converted to JSON-safe primitives before the dict is handed to the response model.
### Issue 2 of 2
nemoguardrails/server/api.py:669-673
`MODIFIED` is silently collapsed into `"success"`, making it impossible for callers to tell whether their content was sanitized (e.g. PII redacted) or passed unchanged. Both `_map_rail_status` and `_build_rails_status` derive blocked/success solely from `RailStatus.BLOCKED` / `rail.stop`, with no representation for the MODIFIED case. A caller who relies on a `"success"` response to treat the content as unmodified would silently forward sanitized content.
```suggestion
def _map_rail_status(status: RailStatus) -> str:
"""Map internal RailStatus to upstream StatusEnum values."""
if status == RailStatus.BLOCKED:
return "blocked"
if status == RailStatus.MODIFIED:
return "modified"
return "success"
```
Reviews (3): Last reviewed commit: ":sparkles: implement `/v1/guardrail/chec..." | Re-trigger Greptile
📝 WalkthroughWalkthroughThis PR introduces a new ChangesGuardrail checks endpoint
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
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 `@nemoguardrails/server/api.py`:
- Line 713: The HTTPException re-raises in api.py should preserve exception
chaining; update the three raise sites (the existing raise
HTTPException(status_code=422, detail=f"Invalid inline config: {ex}") and the
similar raises around lines referenced) to use exception chaining by re-raising
with "from ex" (or "from None" where you intentionally want to suppress context)
so the original traceback is preserved; locate the raise calls in the inline
config parsing/validation handlers (the HTTPException raises at the spots shown
in the diff) and change them to raise HTTPException(...) from ex accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: adb4d588-3aab-48ac-976d-ab512af83b4f
📒 Files selected for processing (5)
nemoguardrails/rails/llm/llmrails.pynemoguardrails/rails/llm/options.pynemoguardrails/server/api.pynemoguardrails/server/schemas/openai.pytests/server/test_guardrail_checks.py
3c5e504 to
359e989
Compare
ebfc23e to
a6b6e9f
Compare
Pouyanpi
left a comment
There was a problem hiding this comment.
Thanks @m-misiura , some comments and questions for you.
I think we should remove the log. If check_async isn’t exposing the right contract for the server as a consumer, let’s discuss that separately (feel free to open an issue and assign it to me.)
there’s always exactly one blocking rail, which we currently report. the rest are either no-ops, passed, or modified.
we don't report modified because there can be multiple rails. the current check logic is intentionally minimal, but we can extend it to support additional scenarios if needed.
If we align on the desired output shape, I think we can simplify this considerably. Let's discuss that
|
@m-misiura also we should return a clear unsupported 4xx for colang 2.0 configs, check_async doesn't support Colang 2.0. |
As usual, thanks for very constructive comments @Pouyanpi WDYT about the following action plan. I will strip this PR down to a thin HTTP surface for {
"status": "passed | modified | blocked",
"content": "text after rails processing",
"rail": "blocking rail name or null"
}
There should be no changes to On the request side: the checks endpoint seem to need Follow-up PRs :
The |
|
@m-misiura thank you. this sounds great! re follow-up PRs, l suggest we discuss them in our meeting. I really like the opportunistic refactoring that you did, let's keep that 👍🏻 |
/v1/guardrail/checks endpoint/v1/guardrail/checks endpoint
2c4cef6 to
2187ab0
Compare
Signed-off-by: m-misiura <mmisiura@redhat.com>
2187ab0 to
6b62cce
Compare
Open review comments need your response@m-misiura this PR is waiting on you. Reply to each open review comment so a reviewer can confirm it is resolved. For every comment, leave a reply that either points to the change you made or explains why no change is needed. Pushing a fix without replying is not enough: reviewers cannot tell which comments a commit addresses, so each thread needs an explicit reply. Review readiness guide: https://github.com/NVIDIA-NeMo/Guardrails/blob/develop/CONTRIBUTING.md#review-readiness |
after resolving llm_rails, we check colang_version != "1.0" and return 422 with "check_async does not support Colang 2.0 configurations." |
Pouyanpi
left a comment
There was a problem hiding this comment.
LGTM, thank you @m-misiura 🚀
Description
This PR add
/v1/guardrail/checksendpoint, wired throughcheck_async()Briefly,
options.py: added log field toRailsResultto surface generation log fromcheck_asyncllmrails.py: threaded log=response.log through all threeRailsResultreturn sites incheck_async()schemas/openai.py: addedGuardrailCheckRequest,GuardrailCheckResponse,RailStatusEntry,GuardrailCheckDataOutputmodels matching upstream OpenAPI specapi.py: endpoint + helpers (_inject_model, _filter_log, _map_rail_status, _build_rails_status). Extracted _inject_model() from _get_rails to share with inline config pathThis PR deals with the following issue
Test Plan
tests/server/test_guardrail_checks.py, tests seem to pass:"options": {"log": {"activated_rails": true}}{ "status": "blocked", "rails_status": { "self check input": { "status": "blocked" } }, "guardrails_data": { "config_ids": [ "checks_live_test" ], "log": { "activated_rails": [], "stats": { "input_rails_duration": 3.91440486907959, "dialog_rails_duration": null, "generation_rails_duration": null, "output_rails_duration": null, "total_duration": 3.9169669151306152, "llm_calls_duration": 3.8865039348602295, "llm_calls_count": 1, "llm_calls_total_prompt_tokens": 144, "llm_calls_total_completion_tokens": 522, "llm_calls_total_tokens": 666 } } } }{ "status": "success", "rails_status": { "self check input": { "status": "success" }, "detect sensitive data on input": { "status": "success" } }, "guardrails_data": { "config_ids": [ "checks_live_test" ], "log": { "activated_rails": [], "stats": { "input_rails_duration": 4.953992128372192, "dialog_rails_duration": null, "generation_rails_duration": null, "output_rails_duration": null, "total_duration": 4.959690093994141, "llm_calls_duration": 4.178013324737549, "llm_calls_count": 1, "llm_calls_total_prompt_tokens": 142, "llm_calls_total_completion_tokens": 590, "llm_calls_total_tokens": 732 } } } }Checklist
cc @Pouyanpi @tgasser-nv
Summary by CodeRabbit
New Features
/v1/guardrail/checksAPI endpoint for validating messages against guardrailsImprovements