Skip to content

Nemo provider bypasses sub request connector admission control and circuit breaking 755 - #827

Draft
rkaplan-hub wants to merge 1 commit into
praxis-proxy:mainfrom
rkaplan-hub:NemoProvider-bypasses-SubRequestConnector-admission-control-and-circuit-breaking-755
Draft

Nemo provider bypasses sub request connector admission control and circuit breaking 755#827
rkaplan-hub wants to merge 1 commit into
praxis-proxy:mainfrom
rkaplan-hub:NemoProvider-bypasses-SubRequestConnector-admission-control-and-circuit-breaking-755

Conversation

@rkaplan-hub

Copy link
Copy Markdown
Contributor

Summary

Route ai_guardrails NeMo callouts through Praxis SubRequestClient instead of a bare reqwest::Client.
NemoProvider was the only remaining sidecar callout that skipped admission control, per-peer circuit breaking, and the unified deadline. This change wires it through the same path as openai_web_search / openai_file_resolve:

  • AiGuardrailsFilter gains from_config_with_client and captures the shared runtime client when the server registry provides one.
  • NemoProvider::evaluate builds a SubRequest and calls praxis_ai_apis::subrequest::execute_url.
  • execute_url is now public so the filters crate can use the existing helper instead of duplicating URL/DNS/deadline logic.
  • Unused reqwest and futures deps are dropped from praxis-ai-filters.
    YAML (endpoint, timeout_ms, model, phase) and verdicts (passed / blocked / modified) are unchanged. This is the smallest complete fix for the gap: one provider, one registration site, no new user-facing config.

Related issue

Closes #755

Validation

  • Unit tests
    • cargo test -p praxis-ai-filters --lib -- guardrails — 44 passed (includes wiremock NeMo callouts over SubRequestClient)
    • cargo test -p praxis-ai-filters --lib -- build_ai_registry — passed
  • Integration or functional tests
    • Existing example tests: cargo test -p praxis-tests-integration -- nemo_guardrails
    • Not run here: praxis-test-utils failed to compile (RunArgs has no shutdown_signal), unrelated to this change.
  • make lint

Checklist

  • I reviewed every changed line and can explain the change.
  • New capabilities include an example config and functional example test.
    • No new YAML fields. Existing examples/configs/nemo-guardrails.yaml and nemo_guardrails_* functional tests still apply.
  • User-facing behavior and generated documentation are updated.
    • Callout transport only; filter docs/YAML contract unchanged.
  • Performance-sensitive changes include appropriate benchmark or load-test evidence.
    • No new hot path. Callouts now inherit the runtime admission semaphore and circuit breaker that already protect other sub-requests. Happy-path unit tests cover pass/block/redact and provider HTTP failures.
  • Commits are signed and include a Signed-off-by trailer.

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review

Summary: Routes NeMo guardrail callouts through SubRequestClient instead of a bare reqwest::Client, inheriting admission control, circuit breaking, and unified deadlines. Registration follows the established shared-client pattern. Clean removal of reqwest streaming body code in favor of execute_url.

Severity Count
Critical 0
Large 0
Medium 1

body.extend_from_slice(&chunk);
/// Reject non-2xx HTTP responses from the provider.
fn ensure_success_status(response: &SubResponse) -> Result<(), FilterError> {
if !(200..300).contains(&(response.status as usize)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Medium] response.status as usize is a lossless cast that will be flagged by the cast_lossless lint (all cast operations are denied workspace-wide). The cast is also unnecessary: !(200..300).contains(&response.status) works directly because Rust infers Range<u16> from the &u16 argument. This matches the identical check in filters/src/callout/mod.rs:327.

if !(200..300).contains(&response.status) {

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.

Done

@rkaplan-hub
rkaplan-hub force-pushed the NemoProvider-bypasses-SubRequestConnector-admission-control-and-circuit-breaking-755 branch 3 times, most recently from a5f641b to 96295a9 Compare September 1, 2026 14:48
…mission-control-and-circuit-breaking-755-rebase

Signed-off-by: kaplan <rkaplan@redhat.com>
@rkaplan-hub
rkaplan-hub force-pushed the NemoProvider-bypasses-SubRequestConnector-admission-control-and-circuit-breaking-755 branch from 96295a9 to 5b00ade Compare September 1, 2026 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ai_guardrails: NemoProvider bypasses SubRequestConnector admission control and circuit breaking

2 participants