feat(guardrails): add NeMo Provider#285
Conversation
|
Missing Signed-off-by: 25b82fc. All commits require sign-off (via |
25b82fc to
3803297
Compare
|
Unsigned commits: 3803297. Please sign your commits. |
05bb6a1 to
97f4123
Compare
praxis-bot
left a comment
There was a problem hiding this comment.
NeMo Provider Review
Solid implementation of the NeMo guardrails provider with correct request-side integration, good error handling structure, and functional integration tests covering all three verdict paths plus provider-down scenarios.
Findings
| # | Severity | File | Summary |
|---|---|---|---|
| 1 | Large | nemo.rs |
Content-Length size check bypassed for chunked responses |
| 2 | Large | nemo.rs |
No unit tests for map_nemo_response or blocked_rail_names |
| 3 | Large | filter.rs |
No unit tests for extract_messages |
| 4 | Large | tests.rs |
Only happy-path on_request_body test; no block/redact/error path tests |
| 5 | Medium | nemo.rs |
Short-form separator comment |
| 6 | Medium | filter.rs |
Short-form separator comment |
Non-inline findings
[Medium] filters/src/guardrails/filter.rs and examples/configs/nemo-guardrails.yaml are both missing POSIX trailing newlines (\ No newline at end of file in the diff). Add a final newline to each.
| /// | ||
| /// Returns [`FilterError`] if the body is not valid JSON or does not | ||
| /// contain a recognizable messages field. | ||
| fn extract_messages(body: &Bytes) -> Result<Vec<serde_json::Value>, FilterError> { |
There was a problem hiding this comment.
[Large] extract_messages has no unit tests. Add tests for:
- Valid body with
"messages"array (happy path) - Invalid JSON (verify error message)
- Valid JSON without a
"messages"key "messages"present but not an array (e.g. a string)- Empty
"messages"array (should succeed, returns empty vec)
97f4123 to
c923966
Compare
55c5f0b to
5a1ad8b
Compare
Signed-off-by: Christina Xu <chrxu@redhat.com>
cc7a6fd to
18925e9
Compare
18925e9 to
f68a92d
Compare
Signed-off-by: Christina Xu <chrxu@redhat.com>
f68a92d to
e37a357
Compare
|
@aslakknutsen thanks so much for the detailed review ! |
| @@ -0,0 +1,52 @@ | |||
| # Guardrails (NeMo) | |||
There was a problem hiding this comment.
docs/developing/conventions.md requires updating examples/README.md when adding example configs. The new nemo-guardrails.yaml is absent from the General configs table, so discoverability lags the implementation.
Refactor of praxis-proxy/praxis#700
Implements the NeMo provider for the ai_guardrails filter, completing request-side evaluation against NeMo Guardrails. Addresses #48.
The following changes were made:
providers/nemo.rsfilter.rs