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.
97f4123 to
c923966
Compare
55c5f0b to
5a1ad8b
Compare
18925e9 to
f68a92d
Compare
f68a92d to
e37a357
Compare
|
@aslakknutsen thanks so much for the detailed review ! |
e37a357 to
2446374
Compare
a7d913a to
bfaf7cf
Compare
|
@christinaexyou You need to sign the commits, besides that it looks good :) |
praxis-bot
left a comment
There was a problem hiding this comment.
Re-review: NeMo Provider (+692 new lines)
Re-review focusing on new code since the last praxis-bot review. Strong progress -- all prior findings addressed or partially addressed, and the new test coverage is thorough.
Prior finding status
| # | Finding | Status |
|---|---|---|
| 1 | Content-Length bypass for chunked responses | Fixed -- incremental streaming body read with 1 MiB cap added (read_response_body) |
| 2 | No unit tests for map_nemo_response / blocked_rail_names |
Partially fixed -- blocked_rail_names has 5 direct tests; map_nemo_response tested indirectly via wiremock but still lacks direct unit tests for absent-content and unknown-status edge cases |
| 3 | No unit tests for extract_messages |
Fixed -- covered by on_request_body tests for invalid JSON, missing messages, non-array, and empty array |
| 4 | Only happy-path on_request_body test |
Fixed -- 12 new test functions covering block, redact, skip, error, and validation paths |
| 5 | Short-form separator in nemo.rs |
Fixed |
| 6 | Short-form separator in filter.rs |
Fixed |
| 7 | Missing trailing newlines (non-inline) | Partially fixed -- filter.rs fixed; nemo-guardrails.yaml still missing POSIX trailing newline |
New findings
| # | Severity | File | Summary |
|---|---|---|---|
| 1 | Medium | filter.rs |
record_verdict doc comment uses // instead of /// on continuation line |
| 2 | Medium | nemo.rs |
read_response_body doc comment makes false claim about function behavior |
5fb1387 to
4932f1e
Compare
b80ca11 to
725049a
Compare
|
please use this as a reference to replace callout client #599 |
Signed-off-by: Christina Xu <chrxu@redhat.com>
e14791e to
0714921
Compare
|
@leseb thanks for your help unblocking my PR ! fyi the NeMoProvider never used |
Signed-off-by: Christina Xu <chrxu@redhat.com>
0714921 to
d2e38c1
Compare
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